From 7c231d5bab754c0274bbfa747141a7598a5bf6cd Mon Sep 17 00:00:00 2001 From: Ashvin Sharma Date: Tue, 8 Aug 2023 13:32:25 +0530 Subject: [PATCH] INFRA-1890 | Ashvin | Fix approval request 403 --- scripts/prometheus_rule_oom.py | 43 +++++++++++++------ .../service/ApprovalRequestServiceImpl.java | 36 +++++++++++++--- .../service/ChangeRequestService.java | 3 ++ .../service/ChangeRequestServiceImpl.java | 5 +++ .../service/TokenRequestService.java | 3 ++ .../service/TokenRequestServiceImpl.java | 5 +++ .../ApprovalRequestServiceImplTest.java | 18 ++++++-- 7 files changed, 92 insertions(+), 21 deletions(-) diff --git a/scripts/prometheus_rule_oom.py b/scripts/prometheus_rule_oom.py index 250e3f81..9a19d44b 100644 --- a/scripts/prometheus_rule_oom.py +++ b/scripts/prometheus_rule_oom.py @@ -43,8 +43,8 @@ def modify_prometheus_rules(api_instance, namespaces): "expr": f'increase(kube_pod_container_status_restarts_total{{namespace="{namespace}", container="{app_name}"}}[10m]) >= 2 AND ignoring(reason) kube_pod_container_status_last_terminated_reason{{namespace="{namespace}", container="{app_name}", reason="OOMKilled"}} > 0', "labels": { "severity": 'critical', - "alertTeam": f'"{app_team}"', - "appName": f'"{app_name}"', + "alertTeam": f'{app_team}', + "appName": f'{app_name}', }, } pod_oom_killed_alert = { @@ -57,8 +57,8 @@ def modify_prometheus_rules(api_instance, namespaces): "expr": f'kube_pod_container_status_restarts_total{{namespace="{namespace}", container="{app_name}"}} - kube_pod_container_status_restarts_total{{namespace="{namespace}", container="{app_name}"}} offset 5m >= 1 AND ignoring(reason) kube_pod_container_status_last_terminated_reason{{namespace="{namespace}", container="{app_name}", reason="OOMKilled"}} > 0', "labels": { "severity": 'warning', - "alertTeam": f'"{app_team}"', - "appName": f'"{app_name}', + "alertTeam": f'{app_team}', + "appName": f'{app_name}', }, } @@ -78,15 +78,32 @@ def modify_prometheus_rules(api_instance, namespaces): if __name__ == "__main__": - # Load the Kubernetes configuration from default location - config.load_kube_config() + kubernetes_contexts = [ + "nonprod.np.navi-tech.in", + "aps1.prod.navi-tech.in", + "aps1.np.navi-gi.in", + "aps1.prod.navi-gi.in", + "aps1.np.navi-sa.in", + "aps1.prod.navi-sa.in", + "aps1.np.navi-pay.in", + "aps1.prod.navi-pay.in", + ] - # Create the API client - api_inst = client.CustomObjectsApi() + for context in kubernetes_contexts: + print(f"Modifying PrometheusRule resources for Kubernetes context: {context}") + try: + # Load the Kubernetes configuration for the current context + config.load_kube_config(context=context) - # Call the function to modify the PrometheusRule resources - v1 = client.CoreV1Api() - ns = [ns.metadata.name for ns in v1.list_namespace().items] - modify_prometheus_rules(api_inst, ns) + # Create the API client + api_inst = client.CustomObjectsApi() - print("PrometheusRule modifications completed.") + # Call the function to modify the PrometheusRule resources + v1 = client.CoreV1Api() + ns = [ns.metadata.name for ns in v1.list_namespace().items] + modify_prometheus_rules(api_inst, ns) + + print(f"PrometheusRule modifications completed for Kubernetes context: {context}") + print("--------------------------------------------------------------") + except Exception as e: + print(f"Error occurred for Kubernetes context {context}: {str(e)}") diff --git a/src/main/java/com/navi/infra/portal/v2/approvalflow/service/ApprovalRequestServiceImpl.java b/src/main/java/com/navi/infra/portal/v2/approvalflow/service/ApprovalRequestServiceImpl.java index aad48ccf..b9f61b94 100644 --- a/src/main/java/com/navi/infra/portal/v2/approvalflow/service/ApprovalRequestServiceImpl.java +++ b/src/main/java/com/navi/infra/portal/v2/approvalflow/service/ApprovalRequestServiceImpl.java @@ -1,5 +1,7 @@ package com.navi.infra.portal.v2.approvalflow.service; +import static com.navi.infra.portal.v2.approvalflow.entity.RequestType.CHANGE_REQUEST; +import static com.navi.infra.portal.v2.approvalflow.entity.RequestType.TOKEN_REQUEST; import static com.navi.infra.portal.v2.changerequest.entity.RequestStatus.APPROVED; import static com.navi.infra.portal.v2.changerequest.entity.RequestStatus.PENDING; import static com.navi.infra.portal.v2.changerequest.entity.RequestStatus.REJECTED; @@ -33,6 +35,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; @@ -92,7 +95,7 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { // check if CR has all approvals, then close and merge. RequestType requestType = approvalRequest.getRequestType(); - if (requestType.equals(RequestType.CHANGE_REQUEST) && hasNoPendingApprovals(requestType, + if (requestType.equals(CHANGE_REQUEST) && hasNoPendingApprovals(requestType, newApproval.getRequestId())) { changeRequestService.approveAndMerge(approvalRequest.getRequestId()); } @@ -144,7 +147,7 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { @Override public List findPendingByTeamIds(List teamIds) { - return repository.findAllPendingByRequestTypeAndTeamIds(RequestType.CHANGE_REQUEST.code, + return repository.findAllPendingByRequestTypeAndTeamIds(CHANGE_REQUEST.code, teamIds); } @@ -172,7 +175,7 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { // check if CR has no pending, then close. RequestType requestType = approvalRequest.getRequestType(); - if (requestType.equals(RequestType.CHANGE_REQUEST) && hasNoPendingApprovals(requestType, + if (requestType.equals(CHANGE_REQUEST) && hasNoPendingApprovals(requestType, newApproval.getRequestId())) { changeRequestService.reject(approvalRequest.getRequestId()); } @@ -232,7 +235,7 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { approvalRequest.getTeamId())); } - var manifest = manifestRepository.findById(approvalRequest.getRequestId()) + var manifest = getManifest(approvalRequest) .orElseThrow(() -> new NotFoundException( "Manifest not found for CR, id: " + approvalRequest.getRequestId())); @@ -243,6 +246,29 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { } } + private Optional getManifest(ApprovalRequest approvalRequest) { + final var requestId = approvalRequest.getRequestId(); + Long manifestId = null; + if (approvalRequest.getRequestType().equals(CHANGE_REQUEST)) { + final var changeRequest = changeRequestService.findById(requestId) + .orElseThrow(() -> new IllegalStateException( + format("No manifest is associated with this Change Request: %d", requestId))); + manifestId = changeRequest.getManifestId(); + } else if (approvalRequest.getRequestType().equals(TOKEN_REQUEST)) { + final var tokenRequest = tokenRequestService.findById(requestId) + .orElseThrow(() -> new IllegalStateException( + format("No manifest is associated with this Token Request: %d", requestId))); + manifestId = tokenRequest.getManifestId(); + } + + if (manifestId == null) { + throw new IllegalStateException( + format("No manifest is associated with this request: %d", requestId)); + } + + return manifestRepository.findById(manifestId); + } + private boolean userHasSameTeamAsApprovalRequest(Long teamId, Long userId) { return teamService.findByUserId(userId) .stream() @@ -283,7 +309,7 @@ public class ApprovalRequestServiceImpl implements ApprovalRequestService { return teamIds.stream() .map(teamId -> new ApprovalRequestBuilder() .setRequestId(changeRequest.getId()) - .setRequestType(RequestType.CHANGE_REQUEST) + .setRequestType(CHANGE_REQUEST) .setTeamId(teamId) .setUpdatedBy(changeRequest.getCreatedBy()) .setStatus(PENDING) diff --git a/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestService.java b/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestService.java index 81c1216c..bc2f3b66 100644 --- a/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestService.java +++ b/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestService.java @@ -3,9 +3,12 @@ package com.navi.infra.portal.v2.changerequest.service; import com.navi.infra.portal.v2.changerequest.dto.ChangeRequestDto; import com.navi.infra.portal.v2.changerequest.entity.ChangeRequest; import java.util.List; +import java.util.Optional; public interface ChangeRequestService { + Optional findById(Long id); + ChangeRequest create(ChangeRequest changeRequest); ChangeRequest close(Long crId, Long userId); diff --git a/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestServiceImpl.java b/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestServiceImpl.java index dfeafd9f..c2f3e96c 100644 --- a/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestServiceImpl.java +++ b/src/main/java/com/navi/infra/portal/v2/changerequest/service/ChangeRequestServiceImpl.java @@ -53,6 +53,11 @@ public class ChangeRequestServiceImpl implements ChangeRequestService { private final ObjectMapper objectMapper; private final MapUtil mapUtil; + + @Override + public Optional findById(Long id) { + return repository.findById(id); + } @Override @Transactional public ChangeRequest create(final ChangeRequest newCr) { diff --git a/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestService.java b/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestService.java index 3c072e7a..02275f79 100644 --- a/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestService.java +++ b/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestService.java @@ -4,9 +4,12 @@ import com.navi.infra.portal.v2.approvalflow.entity.ApprovalRequest; import com.navi.infra.portal.v2.tokenrequest.dto.TokenRequestDTO; import com.navi.infra.portal.v2.tokenrequest.entity.TokenRequest; import java.util.List; +import java.util.Optional; public interface TokenRequestService { + Optional findById(Long id); + Iterable createTokenRequest(TokenRequest jwtTokenRequestDTO, Long userId); List getAllTokenRequests(List teamIds); diff --git a/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestServiceImpl.java b/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestServiceImpl.java index 484680d0..de8f8a44 100644 --- a/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestServiceImpl.java +++ b/src/main/java/com/navi/infra/portal/v2/tokenrequest/service/TokenRequestServiceImpl.java @@ -42,6 +42,11 @@ public class TokenRequestServiceImpl implements TokenRequestService { private final UserService userService; + @Override + public Optional findById(Long id) { + return tokenRequestsRepository.findById(id); + } + @Override public Iterable createTokenRequest(TokenRequest tokenRequest, Long userId) { TokenRequest tokenRequestWithUser = new TokenRequestBuilder().from(tokenRequest) diff --git a/src/test/java/com/navi/infra/portal/v2/changerequest/service/ApprovalRequestServiceImplTest.java b/src/test/java/com/navi/infra/portal/v2/changerequest/service/ApprovalRequestServiceImplTest.java index fb23c56e..4e960eae 100644 --- a/src/test/java/com/navi/infra/portal/v2/changerequest/service/ApprovalRequestServiceImplTest.java +++ b/src/test/java/com/navi/infra/portal/v2/changerequest/service/ApprovalRequestServiceImplTest.java @@ -27,6 +27,7 @@ import com.navi.infra.portal.v2.approvalflow.service.ApprovalRequestServiceImpl; import com.navi.infra.portal.v2.changerequest.entity.ChangeRequestBuilder; import com.navi.infra.portal.v2.exception.NotFoundException; import com.navi.infra.portal.v2.team.TeamService; +import com.navi.infra.portal.v2.tokenrequest.service.TokenRequestService; import java.io.IOException; import java.util.HashMap; import java.util.List; @@ -67,6 +68,12 @@ public class ApprovalRequestServiceImplTest { @Mock private ManifestRepository manifestRepository; + @Mock + private ChangeRequestService changeRequestService; + + @Mock + private TokenRequestService tokenRequestService; + @BeforeEach void setUp() { user1.setId(1L); @@ -84,6 +91,7 @@ public class ApprovalRequestServiceImplTest { teamMap.put("name", team1.getName()); dataMap.put("team", teamMap); + manifest.setId(1L); manifest.setEnvironment("dev"); manifest.setData(dataMap); @@ -117,6 +125,10 @@ public class ApprovalRequestServiceImplTest { final var toBeSavedRequestApproval = new ApprovalRequestBuilder().from(requestApproval) .setStatus(APPROVED).setRequestType(RequestType.CHANGE_REQUEST) .setUpdatedBy(user1.getId()).createApprovalRequest(); + final var changeRequest = new ChangeRequestBuilder() + .setId(1L) + .setManifestId(1L) + .createChangeRequest(); when(repo.findPendingByRequestId(requestApproval.getId())).thenReturn( Optional.of(requestApproval)); @@ -126,10 +138,10 @@ public class ApprovalRequestServiceImplTest { when(repo.findAllPendingByRequestTypeAndRequestId(RequestType.CHANGE_REQUEST.code, requestApproval.getRequestId())).thenReturn(singletonList(new ApprovalRequest())); when(manifestRepository.findById(1L)).thenReturn(Optional.of(manifest)); + when(changeRequestService.findById(1L)).thenReturn(Optional.of(changeRequest)); - - service = new ApprovalRequestServiceImpl(repo, teamService, manifestRepository, null, null, null, - privilegeUtilService, null); + service = new ApprovalRequestServiceImpl(repo, teamService, manifestRepository, null, + changeRequestService, tokenRequestService, privilegeUtilService, null); final var approvedRequest = service.allowApproveRequest(requestApproval.getId(), user1.getId());