diff --git a/src/main/java/org/dependencytrack/model/PolicyViolation.java b/src/main/java/org/dependencytrack/model/PolicyViolation.java index bae68e4b3..f5869a3ff 100644 --- a/src/main/java/org/dependencytrack/model/PolicyViolation.java +++ b/src/main/java/org/dependencytrack/model/PolicyViolation.java @@ -23,13 +23,12 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import io.swagger.v3.oas.annotations.media.Schema; + import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Pattern; import jakarta.validation.constraints.Size; - import javax.jdo.annotations.Column; import javax.jdo.annotations.IdGeneratorStrategy; -import javax.jdo.annotations.Index; import javax.jdo.annotations.PersistenceCapable; import javax.jdo.annotations.Persistent; import javax.jdo.annotations.PrimaryKey; @@ -47,9 +46,6 @@ @PersistenceCapable @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) -// TODO: Add @Unique composite constraint on the fields component, policyCondition, and type. -// The legacy PolicyEngine erroneously allows for duplicates on those fields, but CelPolicyEngine -// will never produce such duplicates. Until we remove the legacy engine, we can't add this constraint. public class PolicyViolation implements Serializable { public enum Type { @@ -69,12 +65,10 @@ public enum Type { @Persistent(defaultFetchGroup = "true") @Column(name = "PROJECT_ID", allowsNull = "false") - @Index(name = "POLICYVIOLATION_PROJECT_IDX") private Project project; @Persistent(defaultFetchGroup = "true") @Column(name = "COMPONENT_ID", allowsNull = "false") - @Index(name = "POLICYVIOLATION_COMPONENT_IDX") private Component component; @Persistent(defaultFetchGroup = "true") diff --git a/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java b/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java index 75625d8c9..ce777f284 100644 --- a/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java +++ b/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java @@ -66,7 +66,6 @@ import org.dependencytrack.util.VulnerabilityUtil; import org.projectnessie.cel.tools.ScriptCreateException; import org.projectnessie.cel.tools.ScriptException; -import org.slf4j.MDC; import java.math.BigDecimal; import java.time.Duration; @@ -86,7 +85,6 @@ import static java.util.Collections.emptyList; import static org.apache.commons.collections4.MultiMapUtils.emptyMultiValuedMap; import static org.apache.commons.lang3.StringUtils.trimToEmpty; -import static org.dependencytrack.common.MdcKeys.MDC_PROJECT_UUID; import static org.dependencytrack.persistence.jdbi.JdbiFactory.withJdbiHandle; import static org.dependencytrack.policy.cel.definition.CelPolicyTypes.TYPE_COMPONENT; import static org.dependencytrack.policy.cel.definition.CelPolicyTypes.TYPE_LICENSE; @@ -143,8 +141,7 @@ public void evaluateProject(final UUID uuid) { final long startTimeNs = System.nanoTime(); try (final var qm = new QueryManager(); - final var celQm = new CelPolicyQueryManager(qm); - var ignoredMdcProjectUuid = MDC.putCloseable(MDC_PROJECT_UUID, uuid.toString())) { + final var celQm = new CelPolicyQueryManager(qm)) { // TODO: Should this entire procedure run in a single DB transaction? // Would be better for atomicity, but could block DB connections for prolonged // period of time for larger projects with many violations. diff --git a/src/main/java/org/dependencytrack/tasks/PolicyEvaluationTask.java b/src/main/java/org/dependencytrack/tasks/PolicyEvaluationTask.java index d1fa80d25..8cabeacb6 100644 --- a/src/main/java/org/dependencytrack/tasks/PolicyEvaluationTask.java +++ b/src/main/java/org/dependencytrack/tasks/PolicyEvaluationTask.java @@ -28,10 +28,16 @@ import org.dependencytrack.model.WorkflowState; import org.dependencytrack.persistence.QueryManager; import org.dependencytrack.policy.cel.CelPolicyEngine; +import org.dependencytrack.util.WaitingLockConfiguration; +import org.slf4j.MDC; -import java.util.UUID; +import java.time.Duration; +import java.time.Instant; +import static org.dependencytrack.common.MdcKeys.MDC_COMPONENT_UUID; +import static org.dependencytrack.common.MdcKeys.MDC_PROJECT_UUID; import static org.dependencytrack.model.WorkflowStep.POLICY_EVALUATION; +import static org.dependencytrack.util.LockProvider.executeWithLockWaiting; /** * A {@link Subscriber} task that executes policy evaluations for {@link Project}s or {@link Component}s. @@ -49,40 +55,63 @@ public PolicyEvaluationTask() { @Override public void inform(final Event e) { - if (e instanceof final ProjectPolicyEvaluationEvent event) { - WorkflowState projectPolicyEvaluationState; - try (final var qm = new QueryManager()) { - projectPolicyEvaluationState = qm.updateStartTimeIfWorkflowStateExists(event.getChainIdentifier(), POLICY_EVALUATION); - try { - evaluateProject(event.getUuid()); - qm.updateWorkflowStateToComplete(projectPolicyEvaluationState); - } catch (Exception ex) { - qm.updateWorkflowStateToFailed(projectPolicyEvaluationState, ex.getMessage()); - LOGGER.error("An unexpected error occurred while evaluating policies for project " + event.getUuid(), ex); - } + try { + if (e instanceof final ProjectPolicyEvaluationEvent event) { + executeWithLockWaiting(createLockConfiguration(event), () -> evaluateProject(event)); + } else if (e instanceof final ComponentPolicyEvaluationEvent event) { + executeWithLockWaiting(createLockConfiguration(event), () -> evaluateComponent(event)); } - } else if (e instanceof final ComponentPolicyEvaluationEvent event) { - WorkflowState componentMetricsEvaluationState; - try (final var qm = new QueryManager()) { - componentMetricsEvaluationState = qm.updateStartTimeIfWorkflowStateExists(event.getChainIdentifier(), POLICY_EVALUATION); - try { - evaluateComponent(event.getUuid()); - qm.updateWorkflowStateToComplete(componentMetricsEvaluationState); - } catch (Exception ex) { - qm.updateWorkflowStateToFailed(componentMetricsEvaluationState, ex.getMessage()); - LOGGER.error("An unexpected error occurred while evaluating policies for component " + event.getUuid(), ex); - } + } catch (Throwable t) { + LOGGER.error("Failed to execute policy evaluation", t); + } + } + + private void evaluateProject(final ProjectPolicyEvaluationEvent event) { + WorkflowState projectPolicyEvaluationState; + try (final var qm = new QueryManager()) { + projectPolicyEvaluationState = qm.updateStartTimeIfWorkflowStateExists(event.getChainIdentifier(), POLICY_EVALUATION); + try (var ignoredMdcProjectUuid = MDC.putCloseable(MDC_PROJECT_UUID, event.getUuid().toString())) { + new CelPolicyEngine().evaluateProject(event.getUuid()); + qm.updateWorkflowStateToComplete(projectPolicyEvaluationState); + } catch (Exception ex) { + qm.updateWorkflowStateToFailed(projectPolicyEvaluationState, ex.getMessage()); + LOGGER.error("An unexpected error occurred while evaluating policies for project " + event.getUuid(), ex); } } } - private void evaluateProject(final UUID uuid) { - new CelPolicyEngine().evaluateProject(uuid); + private void evaluateComponent(final ComponentPolicyEvaluationEvent event) { + WorkflowState componentMetricsEvaluationState; + try (final var qm = new QueryManager()) { + componentMetricsEvaluationState = qm.updateStartTimeIfWorkflowStateExists(event.getChainIdentifier(), POLICY_EVALUATION); + try (var ignoredMdcComponentUuid = MDC.putCloseable(MDC_COMPONENT_UUID, event.getUuid().toString())) { + new CelPolicyEngine().evaluateComponent(event.getUuid()); + qm.updateWorkflowStateToComplete(componentMetricsEvaluationState); + } catch (Exception ex) { + qm.updateWorkflowStateToFailed(componentMetricsEvaluationState, ex.getMessage()); + LOGGER.error("An unexpected error occurred while evaluating policies for component " + event.getUuid(), ex); + } + } } - private void evaluateComponent(final UUID uuid) { - new CelPolicyEngine().evaluateComponent(uuid); + private static WaitingLockConfiguration createLockConfiguration(final ProjectPolicyEvaluationEvent event) { + return new WaitingLockConfiguration( + /* createdAt */ Instant.now(), + /* name */ "%s-project-%s".formatted(PolicyEvaluationTask.class.getSimpleName(), event.getUuid()), + /* lockAtMostFor */ Duration.ofMinutes(5), + /* lockAtLeastFor */ Duration.ZERO, + /* pollInterval */ Duration.ofMillis(100), + /* waitTimeout */ Duration.ofMinutes(5)); + } + private static WaitingLockConfiguration createLockConfiguration(final ComponentPolicyEvaluationEvent event) { + return new WaitingLockConfiguration( + /* createdAt */ Instant.now(), + /* name */ "%s-component-%s".formatted(PolicyEvaluationTask.class.getSimpleName(), event.getUuid()), + /* lockAtMostFor */ Duration.ofMinutes(1), + /* lockAtLeastFor */ Duration.ZERO, + /* pollInterval */ Duration.ofMillis(100), + /* waitTimeout */ Duration.ofMinutes(1)); } } diff --git a/src/main/resources/migration/changelog-v5.6.0.xml b/src/main/resources/migration/changelog-v5.6.0.xml index e16cad418..430e8fb17 100644 --- a/src/main/resources/migration/changelog-v5.6.0.xml +++ b/src/main/resources/migration/changelog-v5.6.0.xml @@ -132,4 +132,15 @@ AND "PROPERTYNAME" = 'nvd.feeds.url'; + + + + + + + + + + + \ No newline at end of file