Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ public record DeploymentRequestView(
List<DeploymentReviewDecisionView> decisions,
/** The human an API-key submitter acted for (#874); null for a human submission. */
UUID onBehalfOfUserId,
String onBehalfOfEmail) {
String onBehalfOfEmail,
/** Mirrors the environment's {@code change_ticket_required} flag (#994). */
boolean changeTicketRequired,
/** The linked change ticket that explains the request's state, or null (#994). */
DeploymentTicketView changeTicket) {

/** Backward-compatible constructor without the #874 on-behalf-of principal. */
public DeploymentRequestView(UUID id, UUID pipelineId, String pipelineName, PipelineProvider provider,
Expand All @@ -65,7 +69,7 @@ public DeploymentRequestView(UUID id, UUID pipelineId, String pipelineName, Pipe
submittedByEmail, version, commitSha, artifactRef, runUrl, externalRunId, metadata,
status, submissionReason, justification, aiAnalysisId, aiRiskLevel, aiRiskScore,
aiSummary, requiredApprovals, scheduledFor, outcome, outcomeReportedAt, outcomeDetail,
createdAt, decisions, null, null);
createdAt, decisions, null, null, false, null);
}

public DeploymentRequestView {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

import java.time.Clock;
import java.time.Instant;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -136,7 +135,7 @@ public DeploymentRequestView confirmExecution(UUID requestId, UUID organizationI
if (request.getSubmissionReason() == SubmissionReason.EMERGENCY_ACCESS) {
metadata.put("change_ticket_bypassed", "break_glass");
} else {
// Releasable implies an APPROVED and no REJECTED ticket, so chooseTicket is non-null.
// Releasable implies an APPROVED and no REJECTED ticket, so the representative ticket is non-null.
metadata.put("change_ticket_key", evaluation.changeTicket().externalKey());
}
}
Expand Down Expand Up @@ -210,7 +209,7 @@ GateEvaluation evaluate(DeploymentRequestEntity request, Instant now) {
if (ticketRequired) {
var tickets = ticketRepository.findAllByDeploymentRequestId(request.getId());
ticketSatisfied = ticketApproved(tickets);
changeTicket = chooseTicket(tickets);
changeTicket = DefaultDeploymentTicketService.representative(tickets);
}
}
return new GateEvaluation(
Expand All @@ -236,23 +235,6 @@ static boolean ticketApproved(List<DeploymentTicketEntity> tickets) {
return approved;
}

/**
* The ticket that explains the verdict: the newest rejected one (it blocks the gate), else the
* newest approved one, else the newest linked ticket, else null.
*/
private static DeploymentTicketView chooseTicket(List<DeploymentTicketEntity> tickets) {
var newestFirst = Comparator.comparing(DeploymentTicketEntity::getCreatedAt,
Comparator.nullsFirst(Comparator.naturalOrder())).reversed();
return tickets.stream()
.filter(t -> REJECTED.equals(t.getDecision()))
.min(newestFirst)
.or(() -> tickets.stream().filter(t -> APPROVED.equals(t.getDecision()))
.min(newestFirst))
.or(() -> tickets.stream().min(newestFirst))
.map(DefaultDeploymentTicketService::toView)
.orElse(null);
}

/** {@code environmentName} is only read on the releasable path, where the row was found. */
record GateEvaluation(boolean releasable, boolean frozen, String freezeReason,
boolean ticketRequired, DeploymentTicketView changeTicket,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.bablsoft.accessflow.deploygov.api.DeploymentRequestSubmissionResult;
import com.bablsoft.accessflow.deploygov.api.DeploymentRequestView;
import com.bablsoft.accessflow.deploygov.api.DeploymentReviewDecisionView;
import com.bablsoft.accessflow.deploygov.api.DeploymentTicketView;
import com.bablsoft.accessflow.deploygov.api.FreezeBehavior;
import com.bablsoft.accessflow.deploygov.api.IllegalDeploymentRequestStateException;
import com.bablsoft.accessflow.deploygov.api.SubmitDeploymentRequestCommand;
Expand All @@ -28,10 +29,12 @@
import com.bablsoft.accessflow.deploygov.internal.persistence.entity.DeploymentEnvironmentEntity;
import com.bablsoft.accessflow.deploygov.internal.persistence.entity.DeploymentPipelineEntity;
import com.bablsoft.accessflow.deploygov.internal.persistence.entity.DeploymentRequestEntity;
import com.bablsoft.accessflow.deploygov.internal.persistence.entity.DeploymentTicketEntity;
import com.bablsoft.accessflow.deploygov.internal.persistence.repo.DeploymentEnvironmentRepository;
import com.bablsoft.accessflow.deploygov.internal.persistence.repo.DeploymentPipelineRepository;
import com.bablsoft.accessflow.deploygov.internal.persistence.repo.DeploymentRequestRepository;
import com.bablsoft.accessflow.deploygov.internal.persistence.repo.DeploymentReviewDecisionRepository;
import com.bablsoft.accessflow.deploygov.internal.persistence.repo.DeploymentTicketRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.dao.DataIntegrityViolationException;
Expand All @@ -42,12 +45,13 @@
import tools.jackson.core.type.TypeReference;
import tools.jackson.databind.ObjectMapper;

import java.time.Clock;
import java.util.LinkedHashMap;
import java.util.List;
import java.time.Clock;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;

/**
* Submission, listing and cancellation of governed deployment requests (#691). Mirrors
Expand All @@ -71,6 +75,7 @@ public class DefaultDeploymentRequestService implements DeploymentRequestService
private final DeploymentPipelineRepository pipelineRepository;
private final DeploymentEnvironmentRepository environmentRepository;
private final DeploymentReviewDecisionRepository decisionRepository;
private final DeploymentTicketRepository ticketRepository;
private final EffectiveDeploymentPermissionResolver permissionResolver;
private final FreezeWindowEvaluator freezeWindowEvaluator;
private final DeploymentRequestStateService stateService;
Expand Down Expand Up @@ -332,7 +337,15 @@ private DeploymentRequestEntity require(UUID id, UUID organizationId) {
}

private PageResponse<DeploymentRequestView> toPage(Page<DeploymentRequestEntity> page) {
var content = page.getContent().stream().map(e -> toView(e, List.of())).toList();
var ids = page.getContent().stream().map(DeploymentRequestEntity::getId).toList();
// One ticket query per page, never one per row (#994).
var ticketsByRequest = ids.isEmpty() ? Map.<UUID, List<DeploymentTicketEntity>>of()
: ticketRepository.findAllByDeploymentRequestIdIn(ids).stream()
.collect(Collectors.groupingBy(DeploymentTicketEntity::getDeploymentRequestId));
var content = page.getContent().stream()
.map(e -> toView(e, List.of(), DefaultDeploymentTicketService.representative(
ticketsByRequest.getOrDefault(e.getId(), List.of()))))
.toList();
return new PageResponse<>(content, page.getNumber(), page.getSize() <= 0 ? 1 : page.getSize(),
page.getTotalElements(), page.getTotalPages());
}
Expand All @@ -351,11 +364,15 @@ private DeploymentRequestView toDetailView(DeploymentRequestEntity entity) {
.map(d -> new DeploymentReviewDecisionView(d.getId(), d.getReviewerId(),
d.getDecision(), d.getComment(), d.getStage(), d.getDecidedAt()))
.toList();
return toView(entity, decisions);
// Unlike the gate, loaded whatever the environment flag: a linked ticket is worth showing.
var changeTicket = DefaultDeploymentTicketService.representative(
ticketRepository.findAllByDeploymentRequestId(entity.getId()));
return toView(entity, decisions, changeTicket);
}

private DeploymentRequestView toView(DeploymentRequestEntity entity,
List<DeploymentReviewDecisionView> decisions) {
List<DeploymentReviewDecisionView> decisions,
DeploymentTicketView changeTicket) {
var pipeline = pipelineRepository.findById(entity.getPipelineId()).orElse(null);
var environment = environmentRepository.findById(entity.getEnvironmentId()).orElse(null);
var analysis = entity.getAiAnalysisId() == null ? null
Expand Down Expand Up @@ -393,7 +410,9 @@ private DeploymentRequestView toView(DeploymentRequestEntity entity,
entity.getCreatedAt(),
decisions,
entity.getOnBehalfOfUserId(),
onBehalfOf != null ? onBehalfOf.email() : null);
onBehalfOf != null ? onBehalfOf.email() : null,
environment != null && environment.isChangeTicketRequired(),
changeTicket);
}

private Map<String, Object> readMetadata(String json) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.springframework.transaction.annotation.Transactional;

import java.time.Clock;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -20,6 +21,7 @@
class DefaultDeploymentTicketService implements DeploymentTicketService {

private static final String APPROVED = "APPROVED";
private static final String REJECTED = "REJECTED";

private final DeploymentTicketRepository repository;
private final Clock clock;
Expand Down Expand Up @@ -89,6 +91,24 @@ public List<DeploymentTicketView> listByDeploymentRequest(UUID deploymentRequest
.toList();
}

/**
* The ticket that explains a request's change-ticket state — the newest rejected one (it blocks
* the gate), else the newest approved one, else the newest linked ticket, else null. Shared by
* the gate (#993) and the request list/detail views (#994).
*/
static DeploymentTicketView representative(List<DeploymentTicketEntity> tickets) {
var newestFirst = Comparator.comparing(DeploymentTicketEntity::getCreatedAt,
Comparator.nullsFirst(Comparator.naturalOrder())).reversed();
return tickets.stream()
.filter(t -> REJECTED.equals(t.getDecision()))
.min(newestFirst)
.or(() -> tickets.stream().filter(t -> APPROVED.equals(t.getDecision()))
.min(newestFirst))
.or(() -> tickets.stream().min(newestFirst))
.map(DefaultDeploymentTicketService::toView)
.orElse(null);
}

static DeploymentTicketView toView(DeploymentTicketEntity entity) {
return new DeploymentTicketView(
entity.getId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.bablsoft.accessflow.deploygov.internal.persistence.entity.DeploymentTicketEntity;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
Expand All @@ -17,4 +18,6 @@ List<DeploymentTicketEntity> findAllByDeploymentRequestIdAndOrganizationIdOrderB
UUID deploymentRequestId, UUID organizationId);

List<DeploymentTicketEntity> findAllByDeploymentRequestId(UUID deploymentRequestId);

List<DeploymentTicketEntity> findAllByDeploymentRequestIdIn(Collection<UUID> deploymentRequestIds);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.bablsoft.accessflow.deploygov.internal.web;

import com.bablsoft.accessflow.deploygov.api.DeploymentTicketView;

/** The change-ticket summary on the gate (#993) and on deployment requests (#994). */
record DeploymentChangeTicketResponse(String system, String externalKey, String url, String status,
String resolution, String decision) {

static DeploymentChangeTicketResponse from(DeploymentTicketView view) {
return view == null ? null : new DeploymentChangeTicketResponse(view.ticketSystem(),
view.externalKey(), view.url(), view.status(), view.resolution(), view.decision());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.bablsoft.accessflow.core.api.QueryStatus;
import com.bablsoft.accessflow.core.api.RiskLevel;
import com.bablsoft.accessflow.deploygov.api.DeploymentGateView;
import com.bablsoft.accessflow.deploygov.api.DeploymentTicketView;

import java.time.Instant;
import java.util.List;
Expand All @@ -21,25 +20,16 @@ public record DeploymentGateResponse(
Instant scheduledFor,
RiskLevel aiRiskLevel,
boolean changeTicketRequired,
ChangeTicket changeTicket) {
DeploymentChangeTicketResponse changeTicket) {

record Approvals(int required, int granted) {
}

record ChangeTicket(String system, String externalKey, String url, String status,
String resolution, String decision) {

static ChangeTicket from(DeploymentTicketView view) {
return view == null ? null : new ChangeTicket(view.ticketSystem(), view.externalKey(),
view.url(), view.status(), view.resolution(), view.decision());
}
}

static DeploymentGateResponse from(DeploymentGateView view) {
return new DeploymentGateResponse(view.requestId(), view.status(), view.releasable(),
new Approvals(view.requiredApprovals(), view.grantedApprovals()),
view.decisions().stream().map(DeploymentReviewDecisionResponse::from).toList(),
view.frozen(), view.freezeReason(), view.scheduledFor(), view.aiRiskLevel(),
view.changeTicketRequired(), ChangeTicket.from(view.changeTicket()));
view.changeTicketRequired(), DeploymentChangeTicketResponse.from(view.changeTicket()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ public record DeploymentRequestResponse(
boolean canReview,
/** The human an API-key submitter acted for (#874); null for a human submission. */
UUID onBehalfOfUserId,
String onBehalfOfEmail) {
String onBehalfOfEmail,
/** Mirrors the environment's flag (#994). */
boolean changeTicketRequired,
/** The linked change ticket that explains the request's state, or null (#994). */
DeploymentChangeTicketResponse changeTicket) {

/** Every path but the detail read: reviewer eligibility is a detail-read concern, never computed here. */
static DeploymentRequestResponse from(DeploymentRequestView view) {
Expand All @@ -61,6 +65,8 @@ static DeploymentRequestResponse from(DeploymentRequestView view, boolean canRev
view.scheduledFor(), view.outcome(), view.outcomeReportedAt(), view.outcomeDetail(),
view.createdAt(),
view.decisions().stream().map(DeploymentReviewDecisionResponse::from).toList(),
canReview, view.onBehalfOfUserId(), view.onBehalfOfEmail());
canReview, view.onBehalfOfUserId(), view.onBehalfOfEmail(),
view.changeTicketRequired(),
DeploymentChangeTicketResponse.from(view.changeTicket()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.within;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

/**
Expand Down Expand Up @@ -111,9 +112,9 @@ void recordsReloadsAndStampsTheDecisionThroughTheService() {
null, null).orElseThrow();
assertThat(progressed.status()).isEqualTo("Review");
assertThat(progressed.decision()).isEqualTo("APPROVED");
// Postgres keeps microseconds; the first view may carry the clock's nanoseconds (Linux).
assertThat(progressed.decidedAt().truncatedTo(ChronoUnit.MICROS))
.isEqualTo(updated.decidedAt().truncatedTo(ChronoUnit.MICROS));
// Postgres rounds to microseconds; the first view may carry the clock's nanoseconds (Linux).
assertThat(progressed.decidedAt())
.isCloseTo(updated.decidedAt(), within(1, ChronoUnit.MICROS));

var listed = ticketService.listByDeploymentRequest(request.getId(), organizationId);
assertThat(listed).singleElement().satisfies(view -> {
Expand Down
Loading
Loading