diff --git a/integrationtests/controller/bundledeployment/suite_test.go b/integrationtests/controller/bundledeployment/suite_test.go index 5841d3046f..998ae2a8ce 100644 --- a/integrationtests/controller/bundledeployment/suite_test.go +++ b/integrationtests/controller/bundledeployment/suite_test.go @@ -1,11 +1,13 @@ package bundledeployment import ( + "bytes" "context" "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + ctrl "sigs.k8s.io/controller-runtime" "github.com/rancher/fleet/integrationtests/utils" "github.com/rancher/fleet/internal/cmd/controller/reconciler" @@ -15,14 +17,16 @@ import ( "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" ) var ( - cancel context.CancelFunc - cfg *rest.Config - ctx context.Context - testenv *envtest.Environment - k8sClient client.Client + cancel context.CancelFunc + cfg *rest.Config + ctx context.Context + testenv *envtest.Environment + k8sClient client.Client + logsBuffer bytes.Buffer namespace string ) @@ -33,6 +37,10 @@ func TestFleet(t *testing.T) { } var _ = BeforeSuite(func() { + // Configure log capture + GinkgoWriter.TeeTo(&logsBuffer) + ctrl.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + ctx, cancel = context.WithCancel(context.TODO()) testenv = utils.NewEnvTest("../../..") diff --git a/integrationtests/controller/bundledeployment/userid_logging_test.go b/integrationtests/controller/bundledeployment/userid_logging_test.go new file mode 100644 index 0000000000..681be6749e --- /dev/null +++ b/integrationtests/controller/bundledeployment/userid_logging_test.go @@ -0,0 +1,129 @@ +package bundledeployment + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/rancher/fleet/integrationtests/utils" + fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +var _ = Describe("UserID logging", func() { + BeforeEach(func() { + logsBuffer.Reset() + }) + + When("BundleDeployment has userID label", func() { + const userID = "test-user-123" + + var ( + bd *fleet.BundleDeployment + namespace string + ) + + BeforeEach(func() { + namespace = "default" + + bd = &fleet.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-bd-", + Namespace: namespace, + Labels: map[string]string{ + fleet.CreatedByUserIDLabel: userID, + }, + }, + Spec: fleet.BundleDeploymentSpec{ + DeploymentID: "test-deployment", + Options: fleet.BundleDeploymentOptions{ + DefaultNamespace: "default", + }, + }, + } + + err := k8sClient.Create(context.Background(), bd) + Expect(err).ToNot(HaveOccurred()) + + // Update status to trigger reconciliation (BundleDeployment controller only reconciles on status changes) + Eventually(func() error { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: bd.Name, Namespace: bd.Namespace}, bd) + if err != nil { + return err + } + bd.Status.Display.State = "TestState" + return k8sClient.Status().Update(context.Background(), bd) + }).Should(Succeed()) + }) + + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), bd)).ToNot(HaveOccurred()) + }) + + It("logs userID in reconciliation", func() { + Eventually(func() string { + return logsBuffer.String() + }).Should(ContainSubstring(bd.Name)) + + logs := logsBuffer.String() + bdLogs := utils.ExtractResourceLogs(logs, bd.Name) + Expect(bdLogs).To(Or( + ContainSubstring(`"userID":"`+userID+`"`), + ContainSubstring(`"userID": "`+userID+`"`), + )) + }) + }) + + When("BundleDeployment does not have userID label", func() { + var ( + bd *fleet.BundleDeployment + namespace string + ) + + BeforeEach(func() { + namespace = "default" + + bd = &fleet.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-bd-no-user-", + Namespace: namespace, + }, + Spec: fleet.BundleDeploymentSpec{ + DeploymentID: "test-deployment", + Options: fleet.BundleDeploymentOptions{ + DefaultNamespace: "default", + }, + }, + } + + err := k8sClient.Create(context.Background(), bd) + Expect(err).ToNot(HaveOccurred()) + + // Update status to trigger reconciliation + Eventually(func() error { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: bd.Name, Namespace: bd.Namespace}, bd) + if err != nil { + return err + } + bd.Status.Display.State = "TestState" + return k8sClient.Status().Update(context.Background(), bd) + }).Should(Succeed()) + }) + + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), bd)).ToNot(HaveOccurred()) + }) + + It("does not log userID in reconciliation", func() { + Eventually(func() string { + return logsBuffer.String() + }).Should(ContainSubstring(bd.Name)) + + logs := logsBuffer.String() + bdLogs := utils.ExtractResourceLogs(logs, bd.Name) + Expect(bdLogs).NotTo(ContainSubstring(`"userID"`)) + }) + }) +}) diff --git a/integrationtests/helmops/controller/suite_test.go b/integrationtests/helmops/controller/suite_test.go index 65990b03b8..f70ae880fd 100644 --- a/integrationtests/helmops/controller/suite_test.go +++ b/integrationtests/helmops/controller/suite_test.go @@ -1,6 +1,7 @@ package controller import ( + "bytes" "context" "testing" "time" @@ -24,6 +25,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" ) const ( @@ -38,6 +40,7 @@ var ( k8sClient client.Client namespace string k8sClientSet *kubernetes.Clientset + logsBuffer bytes.Buffer ) func TestHelmOpsController(t *testing.T) { @@ -50,6 +53,9 @@ var _ = BeforeSuite(func() { ctx, cancel = context.WithCancel(context.TODO()) testEnv = utils.NewEnvTest("../../..") + GinkgoWriter.TeeTo(&logsBuffer) + ctrl.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + var err error cfg, err = utils.StartTestEnv(testEnv) Expect(err).NotTo(HaveOccurred()) diff --git a/integrationtests/helmops/controller/userid_logging_test.go b/integrationtests/helmops/controller/userid_logging_test.go new file mode 100644 index 0000000000..0b3fe724cb --- /dev/null +++ b/integrationtests/helmops/controller/userid_logging_test.go @@ -0,0 +1,91 @@ +package controller + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/rancher/fleet/integrationtests/utils" + "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("HelmOp UserID logging", func() { + var ( + helmop *v1alpha1.HelmOp + namespace string + ) + + createHelmOp := func(name string, labels map[string]string) { + helmop = &v1alpha1.HelmOp{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: labels, + }, + Spec: v1alpha1.HelmOpSpec{ + BundleSpec: v1alpha1.BundleSpec{ + BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{ + DefaultNamespace: "default", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, helmop)).ToNot(HaveOccurred()) + } + + BeforeEach(func() { + var err error + namespace, err = utils.NewNamespaceName() + Expect(err).ToNot(HaveOccurred()) + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + Expect(k8sClient.Create(ctx, ns)).ToNot(HaveOccurred()) + + logsBuffer.Reset() + + DeferCleanup(func() { + Expect(k8sClient.Delete(ctx, helmop)).ToNot(HaveOccurred()) + Expect(k8sClient.Delete(ctx, ns)).ToNot(HaveOccurred()) + }) + }) + + When("HelmOp has user ID label", func() { + const userID = "user-12345" + + BeforeEach(func() { + createHelmOp("test-helmop-with-userid", map[string]string{ + v1alpha1.CreatedByUserIDLabel: userID, + }) + }) + + It("includes userID in log output", func() { + Eventually(func() string { + return logsBuffer.String() + }, timeout).Should(Or( + ContainSubstring(`"userID":"`+userID+`"`), + ContainSubstring(`"userID": "`+userID+`"`), + )) + + logs := logsBuffer.String() + Expect(logs).To(ContainSubstring("HelmOp")) + Expect(logs).To(ContainSubstring(helmop.Name)) + }) + }) + + When("HelmOp does not have user ID label", func() { + BeforeEach(func() { + createHelmOp("test-helmop-without-userid", nil) + }) + + It("does not include userID in log output", func() { + Eventually(func() string { + return logsBuffer.String() + }, timeout).Should(ContainSubstring(helmop.Name)) + + logs := logsBuffer.String() + helmopLogs := utils.ExtractResourceLogs(logs, helmop.Name) + Expect(helmopLogs).NotTo(ContainSubstring(`"userID"`)) + }) + }) +}) diff --git a/internal/cmd/controller/helmops/reconciler/helmop_controller.go b/internal/cmd/controller/helmops/reconciler/helmop_controller.go index 373fb070a0..2fc7e3e6a0 100644 --- a/internal/cmd/controller/helmops/reconciler/helmop_controller.go +++ b/internal/cmd/controller/helmops/reconciler/helmop_controller.go @@ -87,6 +87,12 @@ func (r *HelmOpReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, nil } + if userID := helmop.Labels[fleet.CreatedByUserIDLabel]; userID != "" { + logger = logger.WithValues("userID", userID) + } + + ctx = log.IntoContext(ctx, logger) + // Finalizer handling purgeBundlesFn := func() error { nsName := types.NamespacedName{Name: helmop.Name, Namespace: helmop.Namespace} diff --git a/internal/cmd/controller/reconciler/bundledeployment_controller.go b/internal/cmd/controller/reconciler/bundledeployment_controller.go index 2ccbfb07fd..1ea0a6488c 100644 --- a/internal/cmd/controller/reconciler/bundledeployment_controller.go +++ b/internal/cmd/controller/reconciler/bundledeployment_controller.go @@ -64,6 +64,12 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, client.IgnoreNotFound(err) } + // Enrich logger with userID if present + if userID := bd.Labels[fleet.CreatedByUserIDLabel]; userID != "" { + logger = logger.WithValues("userID", userID) + } + logger.V(1).Info("Reconciling bundledeployment") + // The bundle reconciler takes care of adding the finalizer when creating a bundle deployment if !bd.DeletionTimestamp.IsZero() { if controllerutil.ContainsFinalizer(bd, finalize.BundleDeploymentFinalizer) {