Fixes #25713: ReportsExecution doesn't have timezone on all fields

webapp/sources/rudder/rudder-core/src/main/resources/reportsSchema.sql

@@ -145,8 +145,8 @@ CREATE TABLE ReportsExecution (
 , date         timestamp with time zone NOT NULL
 , nodeConfigId text
 , insertionId  bigint
-, insertiondate timestamp default now()
-, compliancecomputationdate timestamp
+, insertiondate timestamp with time zone default now()
+, compliancecomputationdate timestamp with time zone
 , PRIMARY KEY(nodeId, date)
 );
 

webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala

@@ -50,6 +50,7 @@ import bootstrap.liftweb.checks.consistency.CheckRudderGlobalParameter
 import bootstrap.liftweb.checks.consistency.CloseOpenUserSessions
 import bootstrap.liftweb.checks.migration.CheckAddSpecialNodeGroupsDescription
 import bootstrap.liftweb.checks.migration.CheckRemoveRuddercSetting
+import bootstrap.liftweb.checks.migration.CheckTableReportsExecutionTz
 import bootstrap.liftweb.checks.migration.CheckTableScore
 import bootstrap.liftweb.checks.migration.CheckTableUsers
 import bootstrap.liftweb.checks.migration.MigrateChangeValidationEnforceSchema
@@ -3299,6 +3300,7 @@ object RudderConfigInit {
         inventoryHistoryJdbcRepository,
         KEEP_DELETED_NODE_FACT_DURATION
       ),
+      new CheckTableReportsExecutionTz(doobie),
       new CheckTechniqueLibraryReload(
         techniqueRepositoryImpl,
         uuidGen

webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckTableReportsExecutionTz.scala

@@ -0,0 +1,120 @@
+package bootstrap.liftweb.checks.migration
+
+/*
+ *************************************************************************************
+ * Copyright 2024 Normation SAS
+ *************************************************************************************
+ *
+ * This file is part of Rudder.
+ *
+ * Rudder is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * In accordance with the terms of section 7 (7. Additional Terms.) of
+ * the GNU General Public License version 3, the copyright holders add
+ * the following Additional permissions:
+ * Notwithstanding to the terms of section 5 (5. Conveying Modified Source
+ * Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU General
+ * Public License version 3, when you create a Related Module, this
+ * Related Module is not considered as a part of the work and may be
+ * distributed under the license agreement of your choice.
+ * A "Related Module" means a set of sources files including their
+ * documentation that, without modification of the Source Code, enables
+ * supplementary functions or services in addition to those offered by
+ * the Software.
+ *
+ * Rudder is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Rudder.  If not, see <http://www.gnu.org/licenses/>.
+
+ *
+ *************************************************************************************
+ */
+
+import bootstrap.liftweb.*
+import com.normation.errors.IOResult
+import com.normation.rudder.db.Doobie
+import com.normation.zio.*
+import doobie.Update0
+import doobie.implicits.*
+import doobie.util.fragment.Fragment
+import zio.interop.catz.*
+
+/*
+ * When we created table ReportsExecution, we didn't use "timestamp with time zone" for column
+ * 'insertiondate' and 'compliancecomputationdate'
+ */
+class CheckTableReportsExecutionTz(
+    doobie: Doobie
+) extends BootstrapChecks {
+
+  import CheckTableReportsExecutionTz.*
+  import doobie.*
+
+  override def description: String =
+    "Check if database table ReportsExecution has timestamps with time zone and correct it if not"
+
+  def addTz(table: String, column: String, overrideChange: Option[Fragment] = None): IOResult[Unit] = {
+
+    transactIOResult(s"Error when adding time zone to column ${table}.${column}")(xa =>
+      query(table, column, overrideChange).run.transact(xa)
+    ).unit
+  }
+
+  def migrationProg(table: String, overrideChange: Option[Fragment] = None): IOResult[Unit] = {
+    for {
+      _ <- addTz(table, insertionDateColumn, overrideChange)
+      _ <- addTz(table, complianceComputationDateColumn, overrideChange)
+    } yield ()
+  }
+
+  override def checks(): Unit = {
+    // Actually run the migration async to avoid blocking for that.
+    // There is no need to have it sync.
+    migrationProg(reportsExecution)
+      .catchAll(err => BootstrapLogger.error(s"Error when trying to add time zone to ReportsExecution table: ${err.fullMsg}"))
+      .forkDaemon
+      .runNow
+  }
+
+}
+
+object CheckTableReportsExecutionTz {
+  val reportsExecution:                String = "reportsexecution"
+  val insertionDateColumn:             String = "insertiondate"
+  val complianceComputationDateColumn: String = "compliancecomputationdate"
+
+  // overrideChange is only used for tests
+  def query(table: String, column: String, overrideChange: Option[Fragment] = None): Update0 = {
+    def toQuotedFr(s: String) = Fragment.const("'" + s + "'")
+    def toFr(s:       String) = Fragment.const(s)
+
+    val alter: Fragment = fr"ALTER TABLE " ++ toFr(table) ++ fr" ALTER COLUMN " ++ toFr(
+      column
+    ) ++ fr" TYPE TIMESTAMP WITH TIME ZONE USING " ++ toFr(column) ++ fr" AT TIME ZONE 'UTC';"
+
+    val sql = {
+      fr"""
+        DO $$$$ BEGIN
+          IF NOT EXISTS (
+            select column_name, data_type
+            from information_schema.columns
+            where table_name = """ ++ toQuotedFr(table) ++ fr" and column_name = " ++ toQuotedFr(
+        column
+      ) ++ fr""" and data_type = 'timestamp with time zone'
+          )
+          THEN
+          """ ++ overrideChange.getOrElse(alter) ++ fr"""
+          end if;
+        END $$$$;"""
+    }
+
+    sql.update
+  }
+}

webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/DbCommonMigration.scala

@@ -1,3 +1,40 @@
+/*
+ *************************************************************************************
+ * Copyright 2023 Normation SAS
+ *************************************************************************************
+ *
+ * This file is part of Rudder.
+ *
+ * Rudder is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * In accordance with the terms of section 7 (7. Additional Terms.) of
+ * the GNU General Public License version 3, the copyright holders add
+ * the following Additional permissions:
+ * Notwithstanding to the terms of section 5 (5. Conveying Modified Source
+ * Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU General
+ * Public License version 3, when you create a Related Module, this
+ * Related Module is not considered as a part of the work and may be
+ * distributed under the license agreement of your choice.
+ * A "Related Module" means a set of sources files including their
+ * documentation that, without modification of the Source Code, enables
+ * supplementary functions or services in addition to those offered by
+ * the Software.
+ *
+ * Rudder is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Rudder.  If not, see <http://www.gnu.org/licenses/>.
+
+ *
+ *************************************************************************************
+ */
+
 package bootstrap.liftweb.checks.migration
 
 import doobie.ConnectionIO
@@ -12,8 +49,8 @@ trait DbCommonMigration {
     sql"""
       select count(*)
       from information_schema.columns
-      where table_name = $tableName
-      and column_name = $columnName
+      where table_name = ${tableName}
+      and column_name = ${columnName}
       and is_nullable = 'YES'
     """.query[Int].unique.map(_ > 0)
   }

webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateEventLogEnforceSchema.scala

@@ -75,7 +75,7 @@ class TestMigrateEventLogEnforceSchema extends DBCommon with IOChecker {
   // so we define and use the previous schema without conflicting with the current one (with all values renamed)
   private val previousSchemaDDL = sql"""
     CREATE SEQUENCE eventLogIdSeq_temp START 1;
-    
+
     CREATE TABLE $tempTable (
       id             integer PRIMARY KEY  DEFAULT nextval('eventLogIdSeq_temp')
     , creationDate   timestamp with time zone NOT NULL DEFAULT 'now'
@@ -87,7 +87,7 @@ class TestMigrateEventLogEnforceSchema extends DBCommon with IOChecker {
     , eventType      text
     , data           xml
     );
-    
+
     CREATE INDEX eventType_idx_temp ON $tempTable (eventType);
     CREATE INDEX creationDate_idx_temp ON $tempTable (creationDate);
     CREATE INDEX eventlog_fileFormat_idx_temp ON $tempTable (((((xpath('/entry//@fileFormat',data))[1])::text)));

webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateTableReportsExecution.scala

@@ -0,0 +1,143 @@
+/*
+ *************************************************************************************
+ * Copyright 2024 Normation SAS
+ *************************************************************************************
+ *
+ * This file is part of Rudder.
+ *
+ * Rudder is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * In accordance with the terms of section 7 (7. Additional Terms.) of
+ * the GNU General Public License version 3, the copyright holders add
+ * the following Additional permissions:
+ * Notwithstanding to the terms of section 5 (5. Conveying Modified Source
+ * Versions) and 6 (6. Conveying Non-Source Forms.) of the GNU General
+ * Public License version 3, when you create a Related Module, this
+ * Related Module is not considered as a part of the work and may be
+ * distributed under the license agreement of your choice.
+ * A "Related Module" means a set of sources files including their
+ * documentation that, without modification of the Source Code, enables
+ * supplementary functions or services in addition to those offered by
+ * the Software.
+ *
+ * Rudder is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Rudder.  If not, see <http://www.gnu.org/licenses/>.
+
+ *
+ *************************************************************************************
+ */
+
+package bootstrap.liftweb.checks.migration
+
+import com.normation.rudder.db.DBCommon
+import com.normation.zio.UnsafeRun
+import doobie.*
+import doobie.specs2.IOChecker
+import doobie.syntax.all.*
+import doobie.util.fragments.whereAnd
+import doobie.util.transactor.Transactor
+import org.junit.runner.RunWith
+import org.specs2.runner.JUnitRunner
+import org.specs2.specification.core.Fragments
+import zio.*
+import zio.interop.catz.*
+
+@RunWith(classOf[JUnitRunner])
+class TestMigrateTableReportsExecution extends DBCommon with IOChecker {
+  import CheckTableReportsExecutionTz.*
+
+  val testTable = "reportsexecutiontest"
+
+  private lazy val checkTableReportsExecutionTz = new CheckTableReportsExecutionTz(doobie)
+
+  override def transactor: Transactor[cats.effect.IO] = doobie.xaio
+
+  override def initDb(): Unit = {
+    super.initDb()
+    doobie.transactRunEither(previousSchemaDDL.update.run.transact(_)) match {
+      case Right(_) => ()
+      case Left(ex) => throw ex
+    }
+  }
+
+  // The previous schema, with the renamed table for this test
+  // We need to know for sure the initial state and the final state of the migrated table,
+  // so we define and use the previous schema without conflicting with the current one (with all values renamed)
+  private val previousSchemaDDL = sql"""
+CREATE TABLE reportsexecutiontest (
+  nodeId       text NOT NULL
+, date         timestamp with time zone NOT NULL
+, nodeConfigId text
+, insertionId  bigint
+, insertiondate timestamp default now()
+, compliancecomputationdate timestamp
+, PRIMARY KEY(nodeId, date)
+);
+
+CREATE INDEX reportsexecution_test_date_idx ON reportsexecutiontest (date);
+CREATE INDEX reportsexecution_test_nodeid_nodeconfigid_idx ON reportsexecutiontest (nodeId, nodeConfigId);
+CREATE INDEX reportsexecution_test_uncomputedrun_idx on reportsexecutiontest (compliancecomputationdate) where compliancecomputationdate IS NULL;
+"""
+
+  sequential
+
+  "MigrateEventLogEnforceSchema" should {
+
+    "check migration queries" in {
+      // check that the alter statements are made on existing table and column
+      if (doDatabaseConnection) {
+        check(query(testTable, insertionDateColumn))
+        check(query(testTable, complianceComputationDateColumn))
+      } else Fragments.empty
+    }
+
+    "migrate all columns successfully" in {
+
+      checkTableReportsExecutionTz.migrationProg(testTable).runNow // await for the migration before running assertions test
+
+      val sql = {
+        fr"""SELECT DISTINCT column_name, data_type FROM INFORMATION_SCHEMA.COLUMNS
+          """ ++ whereAnd(
+          fr"table_name = " ++ Fragment.const("'" + testTable + "'"),
+          fr"column_name in (" ++ Fragment.const("'" + insertionDateColumn + "'")
+        ) ++ fr", " ++ Fragment.const("'" + complianceComputationDateColumn + "'") ++ fr")"
+      }
+
+      // check that all migrations are eventually applied even in async mode
+      doobie.transactRunEither(
+        sql
+          .query[(String, String)]
+          .to[List]
+          .transact(_)
+      ) match {
+        case Right(res) =>
+          res must containTheSameElementsAs( // is_nullable is 'NO' for every column
+            List(insertionDateColumn, complianceComputationDateColumn).map((_, "timestamp with time zone"))
+          ).eventually(10, 100.millis.asScala)
+        case Left(ex)   =>
+          ko(
+            s"Migration of 'ReportsExecution' table lead to error : ${ex.getMessage}"
+          )
+      }
+    }
+
+    "a second migration doesn't do the 'if' part" in {
+      val res = {
+        checkTableReportsExecutionTz
+          .migrationProg(testTable, Some(fr"RAISE EXCEPTION 'oh no!'; "))
+          .either
+          .runNow // await for the migration before running assertions test
+      }
+      res must beRight
+    }
+
+  }
+}