INFRA-966 | Saurabh | Kept the existing hard delete flow but added entry in manifest_audit (#1049)

* INFRA-966 | Saurabh | Kept the existing hard delete flow but added entry in manifest_audit

* INFRA-966 | Saurabh | Added test for createAuditFunction

* INFRA-966 | Saurabh | in log added user email

* INFRA-966 | Saurabh | build issue
This commit is contained in:
Saurabh Bhagwan Sathe
2024-07-04 15:52:01 +05:30
committed by Abhishek Katiyar
parent dc645ad65b
commit 0de101acdd
6 changed files with 42 additions and 23 deletions

View File

@@ -69,8 +69,6 @@ public class Manifest extends JsonEntity {
@Column
private String environment;
@Column(name = "is_deleted", nullable = false)
private Boolean isDeleted = false;
@Type(type = "jsonb")
@Column(columnDefinition = "jsonb")

View File

@@ -10,32 +10,21 @@ import org.springframework.stereotype.Repository;
@Repository
public interface ManifestRepository extends JpaRepository<Manifest, Long> {
@Query("SELECT m FROM Manifest m WHERE m.name = :name AND m.isDeleted = false")
List<Manifest> findByName(String name);
@Query("SELECT m FROM Manifest m WHERE m.id = :id AND m.isDeleted = false")
Optional<Manifest> findById(Long id);
@Query("SELECT m FROM Manifest m WHERE m.name = :name "
+ "AND m.environment = :environment AND m.isDeleted = false")
Optional<Manifest> findByNameAndEnvironment(String name, String environment);
@Query("SELECT m FROM Manifest m WHERE m.environment = :environment AND m.isDeleted = false")
List<ManifestName> findIdAndNameAndDataByEnvironment(String environment);
@Query("SELECT m FROM Manifest m WHERE m.isDeleted = false")
List<Manifest> findAll();
@Query("SELECT m FROM Manifest m WHERE m.isDeleted = false")
List<ManifestName> findIdAndNameAndEnvironmentAndDataBy();
@Query("SELECT m FROM Manifest m WHERE m.id = :id AND m.isDeleted = false")
Optional<ManifestName> findManifestNameById(Long id);
@Query(value = "select m.id from manifest m "
+ "join deployment d on m.id = d.manifest_id "
+ "join load_balancer lb on d.id = lb.deployment_id "
+ "where lb.data ->> 'endpoint' = :endpoint and m.is_deleted = false", nativeQuery = true)
+ "where lb.data ->> 'endpoint' = :endpoint ", nativeQuery = true)
Optional<Long> findIdByEndpoint(String endpoint);
}

View File

@@ -509,6 +509,14 @@ public class ManifestService {
}
}
private void auditDeletedManifest(Manifest manifest) {
var lastManifestAudit = manifestAuditService.getLatestByManifestId(manifest.getId())
.orElseThrow(() -> new IllegalStateException(
"No audit found for manifest: " + manifest.getId()));
manifestAuditService.createAudit(manifest, lastManifestAudit.getSecretVersion().toString(),
lastManifestAudit.getSuperSecretVersion().toString());
}
public List<String> delete(Long id, Boolean deleteManifest) {
Manifest manifest = map(manifestRepository.findById(id));
@@ -530,10 +538,9 @@ public class ManifestService {
}
if (deleteManifest) {
result.add(manifest.fullName());
Manifest oldManifest = manifestDeepCopy(manifest);
manifest.setIsDeleted(true);
logManifestDifference(manifest, oldManifest);
auditAndSaveIfManifestChanged(manifest, oldManifest);
log.info("User {} Deleting manifest Object - {}",userService.getCurrentUsername(), manifest.fullName());
auditDeletedManifest(manifest);
manifestRepository.deleteById(id);
} else {
saveManifestWithoutSecrets(manifest);
}

View File

@@ -0,0 +1,14 @@
ALTER TABLE manifest
DROP COLUMN is_deleted;
-- Drop the existing foreign key constraint if it exists
ALTER TABLE manifest_audit
DROP CONSTRAINT IF EXISTS manifest_audit_manifest_id_fk;
-- Add the new foreign key constraint with ON DELETE SET NULL
ALTER TABLE manifest_audit
ADD CONSTRAINT manifest_audit_manifest_id_fk FOREIGN KEY (manifest_id)
REFERENCES manifest(id) ON DELETE SET NULL;

View File

@@ -60,6 +60,7 @@ import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -192,7 +193,7 @@ class ManifestServiceTest {
+ "\"infraVertical\":\"lending\",\"type\":\"deployment\"}";
String manifestResponseString = "{\"version\":null,\"id\":null,"
+ "\"name\":\"test\","
+ "\"environment\":\"dev\",\"isDeleted\":false,\"dynamicConfiguration\":null,"
+ "\"environment\":\"dev\",\"dynamicConfiguration\":null,"
+ "\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\","
+ "\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},"
+ "\"extraResources\":null,\"notification\":null,"
@@ -254,7 +255,7 @@ class ManifestServiceTest {
+ " \"cluster\": \"spike.np.navi-tech.in\"," + "\"infraVertical\": \"lending\"}";
String manifestResponseString =
"{" + "\"version\":null," + "\"id\":null," + "\"name\":\"test\","
+ "\"environment\":\"dev\"," + "\"isDeleted\":false,"
+ "\"environment\":\"dev\","
+ "\"dynamicConfiguration\":null,"
+ "\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},"
+ "\"extraResources\":null," + "\"notification\":null,\"deployment\":"
@@ -269,6 +270,7 @@ class ManifestServiceTest {
+ "\"type\":\"deployment\"}";
Manifest manifest = stringToManifest(manifestRequestString);
when(userService.getCurrentUsername()).thenReturn(currentUser);
when(repo.save(any(Manifest.class))).thenReturn(manifest);
final var request = FindGroupNameRequest.builder()
@@ -302,7 +304,7 @@ class ManifestServiceTest {
String testManifest =
"{\n" + " \"version\": 1,\n" + " \"id\": 2,\n" + " \"name\": \"sa-infra-dev-db\",\n"
+ " \"environment\": \"dev\",\n" + "\"isDeleted\":false,"
+ " \"environment\": \"dev\",\n"
+ " \"extraResources\": {\n"
+ " \"version\": 1,\n" + " \"id\": 2\n" + " },\n"
+ " \"infraVertical\": \"sa\",\n" + " \"environmentVariables\": [\n" + " {\n"
@@ -331,8 +333,12 @@ class ManifestServiceTest {
List<String> result = new ArrayList<String>();
result.add(customManifest.fullName());
assertEquals(result, service.delete(2L, true));
ArgumentCaptor<Manifest> manifestCaptor = ArgumentCaptor.forClass(Manifest.class);
verify(manifestAuditService).createAudit(manifestCaptor.capture(), eq("1"), eq("1"));
assertEquals(customManifest.getId(), manifestCaptor.getValue().getId());
}
@Test
@DisplayName("should not update manifest if no changes")
void shouldNotUpdateManifestIfNoChanges() throws JsonProcessingException {
@@ -342,7 +348,7 @@ class ManifestServiceTest {
mapDiffUtil, changeRequestUtils,
authorizationFilter, null, null);
String manifestRequestString = "{\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"team\":{\"name\":\"Infra\"},\"infraVertical\":\"lending\",\"cluster\":\"spike.np.navi-tech.in\",\"type\":\"deployment\"}";
String manifestResponseString = "{\"version\":null,\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"isDeleted\":false,\"dynamicConfiguration\":null,\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"extraResources\":null,\"notification\":null,\"deployment\":null,\"infraVertical\":\"lending\",\"environmentVariables\":[],\"cluster\":\"spike.np.navi-tech.in\",\"team\":{\"name\":\"Infra\"},\"type\":\"deployment\"}";
String manifestResponseString = "{\"version\":null,\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"dynamicConfiguration\":null,\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"extraResources\":null,\"notification\":null,\"deployment\":null,\"infraVertical\":\"lending\",\"environmentVariables\":[],\"cluster\":\"spike.np.navi-tech.in\",\"team\":{\"name\":\"Infra\"},\"type\":\"deployment\"}";
Manifest manifest = stringToManifest(manifestRequestString);
@@ -356,6 +362,7 @@ class ManifestServiceTest {
assertEquals(manifestResponseString, manifestResponse.getManifest().convertToString());
}
@Test
@DisplayName("should update manifest")
void shouldUpdateManifest() throws JsonProcessingException {
@@ -365,7 +372,7 @@ class ManifestServiceTest {
mapDiffUtil, changeRequestUtils,
authorizationFilter, null, null);
String manifestRequestString = "{\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"team\":{\"name\":\"Infra\"},\"infraVertical\":\"lending\",\"cluster\":\"spike.np.navi-tech.in\",\"type\":\"deployment\"}";
String manifestResponseString = "{\"version\":null,\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"isDeleted\":false,\"dynamicConfiguration\":null,\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"extraResources\":null,\"notification\":null,\"deployment\":null,\"infraVertical\":\"lending\",\"environmentVariables\":[],\"cluster\":\"spike.np.navi-tech.in\",\"team\":{\"name\":\"Infra\"},\"type\":\"deployment\"}";
String manifestResponseString = "{\"version\":null,\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"dynamicConfiguration\":null,\"metadata\":{\"repo\":\"test\",\"language\":\"Java\",\"dataSensitivity\":\"PII_SPI\",\"logCriticality\":\"AccessLogs\",\"disasterRecovery\":\"True\"},\"extraResources\":null,\"notification\":null,\"deployment\":null,\"infraVertical\":\"lending\",\"environmentVariables\":[],\"cluster\":\"spike.np.navi-tech.in\",\"team\":{\"name\":\"Infra\"},\"type\":\"deployment\"}";
String currentManifestString = "{\"id\":1,\"name\":\"test\",\"environment\":\"dev\",\"infraVertical\":\"lending\",\"team\":{\"name\":\"CBP\"}}";
Manifest manifest = stringToManifest(manifestRequestString);
Manifest currentManifest = stringToManifest(currentManifestString);

View File

@@ -0,0 +1,4 @@
package com.navi.infra.portal.service.manifest;
public class User {
}