From 4da4bd3a6cd83ba7cb5de281a3c1ff17ff2b33c9 Mon Sep 17 00:00:00 2001 From: "Francois @fanf42 Armand" Date: Wed, 23 Oct 2024 12:05:23 +0200 Subject: [PATCH] Fixes #25713: ReportsExecution doesn't have timezone on all fields --- .../src/main/resources/reportsSchema.sql | 4 +- .../bootstrap/liftweb/RudderConfig.scala | 2 + .../CheckTableReportsExecutionTz.scala | 120 +++++++++++++++ .../checks/migration/DbCommonMigration.scala | 41 ++++- .../TestMigrateEventLogEnforceSchema.scala | 4 +- .../TestMigrateTableReportsExecution.scala | 143 ++++++++++++++++++ 6 files changed, 308 insertions(+), 6 deletions(-) create mode 100644 webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckTableReportsExecutionTz.scala create mode 100644 webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateTableReportsExecution.scala diff --git a/webapp/sources/rudder/rudder-core/src/main/resources/reportsSchema.sql b/webapp/sources/rudder/rudder-core/src/main/resources/reportsSchema.sql index 16420635d9c..e5e4de3460d 100644 --- a/webapp/sources/rudder/rudder-core/src/main/resources/reportsSchema.sql +++ b/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) ); diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala index 180ba4bdb36..8cd2ca49223 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala +++ b/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 diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckTableReportsExecutionTz.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/CheckTableReportsExecutionTz.scala new file mode 100644 index 00000000000..60973334545 --- /dev/null +++ b/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 . + + * + ************************************************************************************* + */ + +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 + } +} diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/DbCommonMigration.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/DbCommonMigration.scala index 3a0e58b42cb..a2079bc162d 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/DbCommonMigration.scala +++ b/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 . + + * + ************************************************************************************* + */ + 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) } diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateEventLogEnforceSchema.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateEventLogEnforceSchema.scala index 72e4314ef09..6f0f8e5fb68 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateEventLogEnforceSchema.scala +++ b/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))); diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateTableReportsExecution.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateTableReportsExecution.scala new file mode 100644 index 00000000000..3fb30525538 --- /dev/null +++ b/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 . + + * + ************************************************************************************* + */ + +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 + } + + } +}