Skip to content

Commit

Permalink
[PLAT-13995][PLAT-12270][PLAT-14002][PLAT-14011] Tags should be optio…
Browse files Browse the repository at this point in the history
…nal 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
  • Loading branch information
Sahith02 committed May 17, 2024
1 parent 7b3e9e1 commit 7f76a34
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ public Optional<ObjectNode> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,7 +55,7 @@ public class TelemetryProvider extends Model {
@Valid
@DbJson
@ApiModelProperty(value = "Extra Tags", accessMode = READ_WRITE)
private Map<String, String> tags;
private Map<String, String> tags = new HashMap<>();

@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'")
@ApiModelProperty(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -136,4 +139,27 @@ public void validate(TelemetryProvider provider) {
.throwError();
}
}

public boolean isProviderInUse(Customer customer, UUID providerUUID) {
Set<Universe> 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<UniverseLogsExporterConfig> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,5 +21,5 @@ public class UniverseLogsExporterConfig {

@NotNull
@ApiModelProperty(value = "Additional tags", accessMode = READ_WRITE)
private Map<String, String> additionalTags;
private Map<String, String> additionalTags = new HashMap<>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -61,7 +61,7 @@ properties:
log_relation:
description: Log relation
type: boolean
log_row:
log_rows:
description: Log row
type: boolean
log_statement:
Expand Down
6 changes: 3 additions & 3 deletions managed/src/main/resources/swagger-strict.json
Original file line number Diff line number Diff line change
Expand Up @@ -15713,8 +15713,8 @@
"description" : "Log relation",
"type" : "boolean"
},
"logRow" : {
"description" : "Log row",
"logRows" : {
"description" : "Log rows",
"type" : "boolean"
},
"logStatement" : {
Expand All @@ -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" : {
Expand Down
6 changes: 3 additions & 3 deletions managed/src/main/resources/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -15858,8 +15858,8 @@
"description" : "Log relation",
"type" : "boolean"
},
"logRow" : {
"description" : "Log row",
"logRows" : {
"description" : "Log rows",
"type" : "boolean"
},
"logStatement" : {
Expand All @@ -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" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down

0 comments on commit 7f76a34

Please sign in to comment.