From 7f76a343dbea33f43cd4cce27054c2c7f38809d7 Mon Sep 17 00:00:00 2001 From: Sahith Kurapati Date: Thu, 16 May 2024 07:17:17 +0000 Subject: [PATCH] [PLAT-13995][PLAT-12270][PLAT-14002][PLAT-14011] Tags should be optional when creating telemetry provider Summary: This diff fixes the following 4 tickets: 1. [PLAT-13995] Tags should be optional when creating telemetry provider. 2. [PLAT-12270] Rename logRow to logRows + pgaudit.log_row to pgaudit.log_rows in YBA code 3. [PLAT-14002] Should not allow deleting a Telemetry Provider when it's in use 4. [PLAT-14011] Add AuditLog Payload in ModifyAuditLogging task type details For #2, it is okay to change the request body param `logRow` -> `logRows` since this is an internal API and no one uses this DB audit logs feature yet including UI, YBM, other clients, etc. Test Plan: Manually tested all 4 of the above test cases. Run UTs. Run itests. Reviewers: amalyshev Reviewed By: amalyshev Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D35124 --- .../yw/commissioner/Commissioner.java | 5 ++++ .../yugabyte/yw/common/gflags/GFlagsUtil.java | 2 +- .../controllers/CustomerTaskController.java | 5 ++++ .../TelemetryProviderController.java | 25 ++++++++++++++++- .../yugabyte/yw/models/TelemetryProvider.java | 3 +- .../helpers/TelemetryProviderService.java | 28 ++++++++++++++++++- .../audit/UniverseLogsExporterConfig.java | 3 +- .../models/helpers/audit/YSQLAuditConfig.java | 4 +-- .../components/schemas/YSQLAuditConfig.yaml | 4 +-- .../src/main/resources/swagger-strict.json | 6 ++-- managed/src/main/resources/swagger.json | 6 ++-- .../yugabyte/yw/api/v2/UniverseTestBase.java | 2 +- 12 files changed, 77 insertions(+), 16 deletions(-) diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/Commissioner.java b/managed/src/main/java/com/yugabyte/yw/commissioner/Commissioner.java index d628b3f0f00e..97a0b665c9b6 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/Commissioner.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/Commissioner.java @@ -279,6 +279,11 @@ public Optional buildTaskStatus( if (versionNumbers != null && !versionNumbers.isEmpty()) { details.set("versionNumbers", versionNumbers); } + // Add auditLogConfig from the task details if it is present. + // This info is useful to render the UI properly while task is in progress. + if (taskInfo.getTaskParams().has("auditLogConfig")) { + details.set("auditLogConfig", taskInfo.getTaskParams().get("auditLogConfig")); + } responseJson.set("details", details); // Set abortable if eligible. diff --git a/managed/src/main/java/com/yugabyte/yw/common/gflags/GFlagsUtil.java b/managed/src/main/java/com/yugabyte/yw/common/gflags/GFlagsUtil.java index 53d52561dd8c..3703e0d12c03 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/gflags/GFlagsUtil.java +++ b/managed/src/main/java/com/yugabyte/yw/common/gflags/GFlagsUtil.java @@ -709,7 +709,7 @@ private static String getYsqlPgConfCsv(AnsibleConfigureServers.Params taskParams ysqlPgConfCsvEntries.add( encodeBooleanPgAuditFlag("pgaudit.log_relation", ysqlAuditConfig.isLogRelation())); ysqlPgConfCsvEntries.add( - encodeBooleanPgAuditFlag("pgaudit.log_row", ysqlAuditConfig.isLogRow())); + encodeBooleanPgAuditFlag("pgaudit.log_rows", ysqlAuditConfig.isLogRows())); ysqlPgConfCsvEntries.add( encodeBooleanPgAuditFlag("pgaudit.log_statement", ysqlAuditConfig.isLogStatement())); ysqlPgConfCsvEntries.add( diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/CustomerTaskController.java b/managed/src/main/java/com/yugabyte/yw/controllers/CustomerTaskController.java index c79e37cf3957..142e47d76c04 100644 --- a/managed/src/main/java/com/yugabyte/yw/controllers/CustomerTaskController.java +++ b/managed/src/main/java/com/yugabyte/yw/controllers/CustomerTaskController.java @@ -131,6 +131,11 @@ private CustomerTaskFormData buildCustomerTaskFromData( taskData.details = taskProgress.get("details"); } else { ObjectNode details = Json.newObject(); + // Add auditLogConfig from the task details if it is present. + // This info is useful to render the UI properly while task is in progress. + if (taskInfo.getTaskParams().has("auditLogConfig")) { + details.set("auditLogConfig", taskInfo.getTaskParams().get("auditLogConfig")); + } ObjectNode versionNumbers = commissioner.getVersionInfo(task, taskInfo); if (versionNumbers != null && !versionNumbers.isEmpty()) { details.set("versionNumbers", versionNumbers); diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/TelemetryProviderController.java b/managed/src/main/java/com/yugabyte/yw/controllers/TelemetryProviderController.java index debd43a86c4a..fc9bea81275b 100644 --- a/managed/src/main/java/com/yugabyte/yw/controllers/TelemetryProviderController.java +++ b/managed/src/main/java/com/yugabyte/yw/controllers/TelemetryProviderController.java @@ -28,12 +28,14 @@ import java.util.List; import java.util.UUID; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; import play.mvc.Http; import play.mvc.Result; @Api( value = "Telemetry Provider", authorizations = @Authorization(AbstractPlatformController.API_KEY_AUTH)) +@Slf4j public class TelemetryProviderController extends AuthenticatedController { @Inject private TelemetryProviderService telemetryProviderService; @@ -126,7 +128,28 @@ public Result createTelemetryProvider(UUID customerUUID, Http.Request request) { }) public Result deleteTelemetryProvider( UUID customerUUID, UUID providerUUID, Http.Request request) { - Customer.getOrBadRequest(customerUUID); + Customer customer = Customer.getOrBadRequest(customerUUID); + + // Check if telemetry provider exists. + boolean doesTelemetryProviderExist = + telemetryProviderService.checkIfExists(customer.getUuid(), providerUUID); + if (!doesTelemetryProviderExist) { + String errorMessage = + String.format("Telemetry Provider '%s' does not exist.", providerUUID.toString()); + log.error(errorMessage); + throw new PlatformServiceException(BAD_REQUEST, errorMessage); + } + + // Check if telemetry provider is in use. + boolean isProviderInUse = telemetryProviderService.isProviderInUse(customer, providerUUID); + if (isProviderInUse) { + String errorMessage = + String.format( + "Cannot delete Telemetry Provider '%s', as it is in use.", providerUUID.toString()); + log.error(errorMessage); + throw new PlatformServiceException(BAD_REQUEST, errorMessage); + } + telemetryProviderService.delete(providerUUID); auditService() .createAuditEntryWithReqBody( diff --git a/managed/src/main/java/com/yugabyte/yw/models/TelemetryProvider.java b/managed/src/main/java/com/yugabyte/yw/models/TelemetryProvider.java index 05c057d6485f..6e4406404fe9 100644 --- a/managed/src/main/java/com/yugabyte/yw/models/TelemetryProvider.java +++ b/managed/src/main/java/com/yugabyte/yw/models/TelemetryProvider.java @@ -17,6 +17,7 @@ import jakarta.persistence.Entity; import jakarta.persistence.Id; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -54,7 +55,7 @@ public class TelemetryProvider extends Model { @Valid @DbJson @ApiModelProperty(value = "Extra Tags", accessMode = READ_WRITE) - private Map tags; + private Map tags = new HashMap<>(); @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'") @ApiModelProperty( diff --git a/managed/src/main/java/com/yugabyte/yw/models/helpers/TelemetryProviderService.java b/managed/src/main/java/com/yugabyte/yw/models/helpers/TelemetryProviderService.java index 44c4f7406533..c5c23a739659 100644 --- a/managed/src/main/java/com/yugabyte/yw/models/helpers/TelemetryProviderService.java +++ b/managed/src/main/java/com/yugabyte/yw/models/helpers/TelemetryProviderService.java @@ -14,7 +14,11 @@ import com.yugabyte.yw.common.BeanValidator; import com.yugabyte.yw.common.PlatformServiceException; +import com.yugabyte.yw.forms.UniverseDefinitionTaskParams.UserIntent; +import com.yugabyte.yw.models.Customer; import com.yugabyte.yw.models.TelemetryProvider; +import com.yugabyte.yw.models.Universe; +import com.yugabyte.yw.models.helpers.audit.UniverseLogsExporterConfig; import io.ebean.annotation.Transactional; import java.util.Collection; import java.util.Collections; @@ -83,7 +87,6 @@ public TelemetryProvider getOrBadRequest(UUID customerUUID, UUID uuid) { public boolean checkIfExists(UUID customerUUID, UUID uuid) { try { TelemetryProvider provider = getOrBadRequest(customerUUID, uuid); - ; if (provider != null) { return true; } @@ -136,4 +139,27 @@ public void validate(TelemetryProvider provider) { .throwError(); } } + + public boolean isProviderInUse(Customer customer, UUID providerUUID) { + Set allUniverses = Universe.getAllWithoutResources(customer); + + // Iterate through all universe details and check if any of them have an audit log config. + for (Universe universe : allUniverses) { + UserIntent primaryUserIntent = universe.getUniverseDetails().getPrimaryCluster().userIntent; + + if (primaryUserIntent.getAuditLogConfig() != null + && primaryUserIntent.getAuditLogConfig().getUniverseLogsExporterConfig() != null) { + List universeLogsExporterConfigs = + primaryUserIntent.getAuditLogConfig().getUniverseLogsExporterConfig(); + + // Check if the provider is in the list of export configs in the audit log config. + for (UniverseLogsExporterConfig config : universeLogsExporterConfigs) { + if (config != null && providerUUID.equals(config.getExporterUuid())) { + return true; + } + } + } + } + return false; + } } diff --git a/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/UniverseLogsExporterConfig.java b/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/UniverseLogsExporterConfig.java index dc1f7a8096ea..5fa78408b6b7 100644 --- a/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/UniverseLogsExporterConfig.java +++ b/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/UniverseLogsExporterConfig.java @@ -5,6 +5,7 @@ import io.swagger.annotations.ApiModel; import io.swagger.annotations.ApiModelProperty; +import java.util.HashMap; import java.util.Map; import java.util.UUID; import javax.validation.constraints.NotNull; @@ -20,5 +21,5 @@ public class UniverseLogsExporterConfig { @NotNull @ApiModelProperty(value = "Additional tags", accessMode = READ_WRITE) - private Map additionalTags; + private Map additionalTags = new HashMap<>(); } diff --git a/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/YSQLAuditConfig.java b/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/YSQLAuditConfig.java index 9ad0f31b7bfd..074573bd76fe 100644 --- a/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/YSQLAuditConfig.java +++ b/managed/src/main/java/com/yugabyte/yw/models/helpers/audit/YSQLAuditConfig.java @@ -46,8 +46,8 @@ public class YSQLAuditConfig { private boolean logRelation; @NotNull - @ApiModelProperty(value = "Log row", accessMode = READ_WRITE) - private boolean logRow; + @ApiModelProperty(value = "Log rows", accessMode = READ_WRITE) + private boolean logRows; @NotNull @ApiModelProperty(value = "Log statement", accessMode = READ_WRITE) diff --git a/managed/src/main/resources/openapi/components/schemas/YSQLAuditConfig.yaml b/managed/src/main/resources/openapi/components/schemas/YSQLAuditConfig.yaml index 05cad9792386..1d9d1fc746e9 100644 --- a/managed/src/main/resources/openapi/components/schemas/YSQLAuditConfig.yaml +++ b/managed/src/main/resources/openapi/components/schemas/YSQLAuditConfig.yaml @@ -9,7 +9,7 @@ required: - log_parameter - log_parameter_max_size - log_relation - - log_row + - log_rows - log_statement - log_statement_once description: YSQL Audit Logging Configuration @@ -61,7 +61,7 @@ properties: log_relation: description: Log relation type: boolean - log_row: + log_rows: description: Log row type: boolean log_statement: diff --git a/managed/src/main/resources/swagger-strict.json b/managed/src/main/resources/swagger-strict.json index 1e37a635b38f..187386578dee 100644 --- a/managed/src/main/resources/swagger-strict.json +++ b/managed/src/main/resources/swagger-strict.json @@ -15713,8 +15713,8 @@ "description" : "Log relation", "type" : "boolean" }, - "logRow" : { - "description" : "Log row", + "logRows" : { + "description" : "Log rows", "type" : "boolean" }, "logStatement" : { @@ -15726,7 +15726,7 @@ "type" : "boolean" } }, - "required" : [ "classes", "enabled", "logCatalog", "logClient", "logLevel", "logParameter", "logParameterMaxSize", "logRelation", "logRow", "logStatement", "logStatementOnce" ], + "required" : [ "classes", "enabled", "logCatalog", "logClient", "logLevel", "logParameter", "logParameterMaxSize", "logRelation", "logRows", "logStatement", "logStatementOnce" ], "type" : "object" }, "YbcThrottleParameters" : { diff --git a/managed/src/main/resources/swagger.json b/managed/src/main/resources/swagger.json index f103476d610c..5d8a1928bbf6 100644 --- a/managed/src/main/resources/swagger.json +++ b/managed/src/main/resources/swagger.json @@ -15858,8 +15858,8 @@ "description" : "Log relation", "type" : "boolean" }, - "logRow" : { - "description" : "Log row", + "logRows" : { + "description" : "Log rows", "type" : "boolean" }, "logStatement" : { @@ -15871,7 +15871,7 @@ "type" : "boolean" } }, - "required" : [ "classes", "enabled", "logCatalog", "logClient", "logLevel", "logParameter", "logParameterMaxSize", "logRelation", "logRow", "logStatement", "logStatementOnce" ], + "required" : [ "classes", "enabled", "logCatalog", "logClient", "logLevel", "logParameter", "logParameterMaxSize", "logRelation", "logRows", "logStatement", "logStatementOnce" ], "type" : "object" }, "YbcThrottleParameters" : { diff --git a/managed/src/test/java/com/yugabyte/yw/api/v2/UniverseTestBase.java b/managed/src/test/java/com/yugabyte/yw/api/v2/UniverseTestBase.java index 3b8cdb971445..0504f1c7820e 100644 --- a/managed/src/test/java/com/yugabyte/yw/api/v2/UniverseTestBase.java +++ b/managed/src/test/java/com/yugabyte/yw/api/v2/UniverseTestBase.java @@ -814,7 +814,7 @@ private void validateYsqlAuditConfig( assertThat( v2YsqlAuditConfig.getLogParameterMaxSize(), is(dbYsqlAuditConfig.getLogParameterMaxSize())); assertThat(v2YsqlAuditConfig.getLogRelation(), is(dbYsqlAuditConfig.isLogRelation())); - assertThat(v2YsqlAuditConfig.getLogRow(), is(dbYsqlAuditConfig.isLogRow())); + assertThat(v2YsqlAuditConfig.getLogRows(), is(dbYsqlAuditConfig.isLogRows())); assertThat(v2YsqlAuditConfig.getLogStatement(), is(dbYsqlAuditConfig.isLogStatement())); assertThat(v2YsqlAuditConfig.getLogStatementOnce(), is(dbYsqlAuditConfig.isLogStatementOnce())); }