Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

break out okta endpoint from health check #7891

Merged
merged 18 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
@@ -1,8 +1,6 @@
package gov.cdc.usds.simplereport.api.heathcheck;

import com.okta.sdk.resource.client.ApiException;
import gov.cdc.usds.simplereport.db.repository.FeatureFlagRepository;
import gov.cdc.usds.simplereport.idp.repository.OktaRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.hibernate.exception.JDBCConnectionException;
Expand All @@ -15,28 +13,16 @@
@RequiredArgsConstructor
public class BackendAndDatabaseHealthIndicator implements HealthIndicator {
private final FeatureFlagRepository _ffRepo;
private final OktaRepository _oktaRepo;
public static final String ACTIVE_LITERAL = "ACTIVE";

@Override
public Health health() {
try {
_ffRepo.findAll();
String oktaStatus = _oktaRepo.getApplicationStatusForHealthCheck();

if (!ACTIVE_LITERAL.equals(oktaStatus)) {
log.info("Okta status didn't return ACTIVE, instead returned " + oktaStatus);
return Health.down().build();
}
return Health.up().build();

// reach into the ff repository returned a bad value or db connection issue respectively
} catch (IllegalArgumentException | JDBCConnectionException e) {
return Health.down().build();
} catch (ApiException e) {
// Okta API call errored
log.info(e.getMessage());
return Health.down().build();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package gov.cdc.usds.simplereport.api.heathcheck;

import gov.cdc.usds.simplereport.idp.repository.OktaRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.HealthIndicator;
import org.springframework.stereotype.Component;

@Component
@Slf4j
@RequiredArgsConstructor
public class OktaHealthIndicator implements HealthIndicator {
private final OktaRepository _oktaRepo;
public static final String ACTIVE_LITERAL = "ACTIVE";

@Override
public Health health() {
try {
String oktaStatus = _oktaRepo.getApplicationStatusForHealthCheck();
if (!ACTIVE_LITERAL.equals(oktaStatus)) {
log.info("Okta status didn't return ACTIVE, instead returned " + oktaStatus);
Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED");
return oktaDegradedWarning.build();
}
} catch (NullPointerException e) {
fzhao99 marked this conversation as resolved.
Show resolved Hide resolved
log.info("Call to Okta repository status returned null");
Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED");
return oktaDegradedWarning.build();
}

return Health.up().build();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package gov.cdc.usds.simplereport.idp.repository;

import static gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator.ACTIVE_LITERAL;
import static gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator.ACTIVE_LITERAL;

import com.okta.sdk.resource.model.UserStatus;
import gov.cdc.usds.simplereport.api.CurrentTenantDataAccessContextHolder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
Expand Down Expand Up @@ -690,7 +691,7 @@ public PartialOktaUser findUser(String username) {

@Override
public String getApplicationStatusForHealthCheck() {
return app.getStatus().toString();
return Objects.requireNonNull(app.getStatus()).toString();
fzhao99 marked this conversation as resolved.
Show resolved Hide resolved
}

private Optional<OrganizationRoleClaims> getOrganizationRoleClaimsFromAuthorities(
Expand Down
7 changes: 7 additions & 0 deletions backend/src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ management:
endpoint.info.enabled: true
endpoints.web.exposure.include: health, info
endpoint.health.show-components: always
endpoint:
health:
show-components: always
status:
http-mapping:
okta_degraded: 204
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pedantic nerd inside me debates the correctness of returning a 204/NO CONTENT. That's not to say that there is an existing 2xx status code that would be correct. But I'd almost be tempted to use a 2xx that doesn't have an existing status (like 299 for example).

At the end of the day I don't think it matters.

probes.enabled: true
okta:
oauth2:
issuer: https://hhs-prime.okta.com/oauth2/default
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package gov.cdc.usds.simplereport.api.healthcheck;

import static gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator.ACTIVE_LITERAL;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.when;

import gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator;
import gov.cdc.usds.simplereport.db.repository.BaseRepositoryTest;
import gov.cdc.usds.simplereport.db.repository.FeatureFlagRepository;
import gov.cdc.usds.simplereport.idp.repository.OktaRepository;
import java.sql.SQLException;
import java.util.List;
import lombok.RequiredArgsConstructor;
Expand All @@ -23,14 +21,12 @@
class BackendAndDatabaseHealthIndicatorTest extends BaseRepositoryTest {

@SpyBean private FeatureFlagRepository mockFeatureFlagRepo;
@SpyBean private OktaRepository mockOktaRepo;

@Autowired private BackendAndDatabaseHealthIndicator indicator;

@Test
void health_succeedsWhenReposDoesntThrow() {
when(mockFeatureFlagRepo.findAll()).thenReturn(List.of());
when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn(ACTIVE_LITERAL);

assertThat(indicator.health()).isEqualTo(Health.up().build());
}
Expand All @@ -51,10 +47,4 @@ void health_failsWhenFeatureFlagRepoThrows() {
when(mockFeatureFlagRepo.findAll()).thenThrow(dbConnectionException);
assertThat(indicator.health()).isEqualTo(Health.down().build());
}

@Test
void health_failsWhenOktaRepoDoesntReturnActive() {
when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("INACTIVE");
assertThat(indicator.health()).isEqualTo(Health.down().build());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package gov.cdc.usds.simplereport.api.healthcheck;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.when;

import gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator;
import gov.cdc.usds.simplereport.db.repository.BaseRepositoryTest;
import gov.cdc.usds.simplereport.idp.repository.DemoOktaRepository;
import lombok.RequiredArgsConstructor;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.test.mock.mockito.SpyBean;

@RequiredArgsConstructor
@EnableConfigurationProperties
class OktaHealthIndicatorTest extends BaseRepositoryTest {

@SpyBean private DemoOktaRepository mockOktaRepo;

@Autowired private OktaHealthIndicator indicator;

@Test
void health_SUCCEEDSWhenOktaRepoReturnsActive() {
when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("ACTIVE");
assertThat(indicator.health()).isEqualTo(Health.up().build());
}

@Test
void health_failsWhenOktaRepoDoesntReturnActive() {
when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("INACTIVE");
Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED");

assertThat(indicator.health()).isEqualTo(oktaDegradedWarning.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import gov.cdc.usds.simplereport.api.CurrentTenantDataAccessContextHolder;
import gov.cdc.usds.simplereport.api.WebhookContextHolder;
import gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator;
import gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator;
import gov.cdc.usds.simplereport.api.pxp.CurrentPatientContextHolder;
import gov.cdc.usds.simplereport.config.AuditingConfig;
import gov.cdc.usds.simplereport.config.AuthorizationProperties;
Expand Down Expand Up @@ -105,6 +106,7 @@
TenantDataAccessService.class,
PatientSelfRegistrationLinkService.class,
BackendAndDatabaseHealthIndicator.class,
OktaHealthIndicator.class,
EmailService.class,
SendGridDisabledConfiguration.class,
})
Expand Down
24 changes: 21 additions & 3 deletions frontend/deploy-smoke.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// endpoint which does a simple ping to a non-sensitive DB table to verify
// all the connections are good.
// https://github.com/CDCgov/prime-simplereport/pull/7057

require("dotenv").config();
let { Builder } = require("selenium-webdriver");
const Chrome = require("selenium-webdriver/chrome");
Expand Down Expand Up @@ -33,16 +32,35 @@ driver
return value;
})
.then((value) => {
if (value.includes("success")) {
let appStatusSuccess, oktaStatusSuccess;
if (value.includes("App status returned success")) {
fzhao99 marked this conversation as resolved.
Show resolved Hide resolved
appStatusSuccess = true;
}
if (value.includes("Okta status returned success")) {
oktaStatusSuccess = true;
}
if (value.includes("App status returned failure")) {
appStatusSuccess = false;
}
if (value.includes("Okta status returned failure")) {
oktaStatusSuccess = false;
}

if (appStatusSuccess && oktaStatusSuccess) {
console.log(`Smoke test returned success status for ${appUrl}`);
process.exitCode = 0;
return;
}
if (value.includes("failure")) {

if (appStatusSuccess === false || oktaStatusSuccess === false) {
console.log(`Smoke test returned failure status for ${appUrl}`);
console.log(
`App health returned ${appStatusSuccess}, okta health returned ${oktaStatusSuccess}`
);
process.exitCode = 1;
return;
}

console.log("Smoke test encountered unknown failure.");
console.log(`Root element value was: ${value}`);
process.exitCode = 1;
Expand Down
40 changes: 33 additions & 7 deletions frontend/src/app/DeploySmokeTest.test.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,56 @@
import { render, screen, waitFor } from "@testing-library/react";
import { FetchMock } from "jest-fetch-mock";

import DeploySmokeTest from "./DeploySmokeTest";
import DeploySmokeTest, {
APP_STATUS_FAILURE,
APP_STATUS_LOADING,
APP_STATUS_SUCCESS,
OKTA_STATUS_LOADING,
OKTA_STATUS_SUCCESS,
} from "./DeploySmokeTest";
import { generateBackendApiHealthResponse } from "./deploySmokeTestTestConstants";

describe("DeploySmokeTest", () => {
beforeEach(() => {
(fetch as FetchMock).resetMocks();
});

it("renders success when returned from the API endpoint", async () => {
(fetch as FetchMock).mockResponseOnce(JSON.stringify({ status: "UP" }));
(fetch as FetchMock).mockResponseOnce(
JSON.stringify(generateBackendApiHealthResponse("UP"))
);
(fetch as FetchMock).mockResponseOnce(
JSON.stringify(generateBackendApiHealthResponse("DOWN"))
);

render(<DeploySmokeTest />);
await waitFor(() =>
expect(screen.queryByText("Status loading...")).not.toBeInTheDocument()
expect(screen.queryByText(APP_STATUS_LOADING)).not.toBeInTheDocument()
);
expect(screen.getByText("Status returned success :)"));
expect(screen.getByText(APP_STATUS_SUCCESS));
});

it("renders failure when returned from the API endpoint", async () => {
(fetch as FetchMock).mockResponseOnce(JSON.stringify({ status: "DOWN" }));
(fetch as FetchMock).mockResponse(
JSON.stringify(generateBackendApiHealthResponse("DOWN"))
fzhao99 marked this conversation as resolved.
Show resolved Hide resolved
);

render(<DeploySmokeTest />);
await waitFor(() =>
expect(screen.queryByText(APP_STATUS_LOADING)).not.toBeInTheDocument()
);
expect(screen.getByText(APP_STATUS_FAILURE));
});

it("renders Okta success when returned from the API endpoint", async () => {
(fetch as FetchMock).mockResponse(
JSON.stringify(generateBackendApiHealthResponse())
);

render(<DeploySmokeTest />);
await waitFor(() =>
expect(screen.queryByText("Status loading...")).not.toBeInTheDocument()
expect(screen.queryByText(OKTA_STATUS_LOADING)).not.toBeInTheDocument()
);
expect(screen.getByText("Status returned failure :("));
expect(screen.getByText(OKTA_STATUS_SUCCESS));
});
});
Loading
Loading