-
Notifications
You must be signed in to change notification settings - Fork 3
feat: s3 health check #723
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
| **Property** | **Description** | **Example** | **Env** | | ||
|--------------------------|-------------------------------------------------------|------------------------------------------------------------------|------------------| | ||
| `s3.bucketName` _string_ | The name of the bucket containing the desired object. | myphotos | `S3_BUCKET_NAME` | | ||
| `s3.endpoint` _string_ | The endpoint either with or without the protocol | https://s3.eu-west-1.amazonaws.com or s3.eu-west-1.amazonaws.com | `S3_ENDPOINT` | | ||
| `s3.region` _string_ | The region to use for SigV4 signing of requests | eu-west-1 | `S3_REGION` | | ||
| `s3.secretKey` _string_ | The AWS secret access key | s3cret | `S3_SECRET_KEY` | | ||
| `s3.accessKey` _string_ | The AWS access key | s3cretAccess | `S3_ACCESS_KEY` | | ||
| **Property** | **Description** | **Example** | **Env** | | ||
|------------------------------------------|-------------------------------------------------------|------------------------------------------------------------------|--------------------------------| | ||
| `s3.bucketName` _string_ | The name of the bucket containing the desired object. | myphotos | `S3_BUCKET_NAME` | | ||
| `s3.endpoint` _string_ | The endpoint either with or without the protocol | https://s3.eu-west-1.amazonaws.com or s3.eu-west-1.amazonaws.com | `S3_ENDPOINT` | | ||
| `s3.region` _string_ | The region to use for SigV4 signing of requests | eu-west-1 | `S3_REGION` | | ||
| `s3.secretKey` _string_ | The AWS secret access key | s3cret | `S3_SECRET_KEY` | | ||
| `s3.accessKey` _string_ | The AWS access key | s3cretAccess | `S3_ACCESS_KEY` | | ||
| `management.health.s3.enabled` _boolean_ | Flag to enable s3 health check. | `true` | `MANAGEMENT_HEALTH_S3_ENABLED` | |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,12 @@ | ||
dependencies { | ||
implementation "org.springframework.boot:spring-boot-autoconfigure" | ||
api 'org.springframework.boot:spring-boot-starter-actuator' | ||
api "io.awspring.cloud:spring-cloud-aws-s3" | ||
|
||
testImplementation 'org.springframework.boot:spring-boot-starter-test' | ||
testImplementation project(":sda-commons-starter-web") | ||
testImplementation project(':sda-commons-web-testing') | ||
testImplementation 'io.github.robothy:local-s3-rest' | ||
testImplementation 'org.junit-pioneer:junit-pioneer:2.2.0', { | ||
exclude group: 'org.junit' | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Copyright 2022- SDA SE Open Industry Solutions (https://www.sda.se) | ||
* | ||
* Use of this source code is governed by an MIT-style | ||
* license that can be found in the LICENSE file or at | ||
* https://opensource.org/licenses/MIT. | ||
*/ | ||
package org.sdase.commons.spring.boot.s3.config; | ||
|
||
import java.util.Map; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator; | ||
import org.springframework.boot.actuate.health.AbstractHealthIndicator; | ||
import org.springframework.boot.actuate.health.Health; | ||
import org.springframework.stereotype.Component; | ||
import software.amazon.awssdk.services.s3.S3Client; | ||
import software.amazon.awssdk.services.s3.model.HeadBucketRequest; | ||
|
||
@Component("s3") | ||
@ConditionalOnEnabledHealthIndicator("s3") | ||
public class S3HealthIndicator extends AbstractHealthIndicator { | ||
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(S3HealthIndicator.class); | ||
|
||
private final S3Client s3Client; | ||
private final String bucketName; | ||
|
||
public S3HealthIndicator( | ||
S3Client getAmazonS3Client, @Value("${s3.bucketName}") String bucketName) { | ||
this.s3Client = getAmazonS3Client; | ||
this.bucketName = bucketName; | ||
} | ||
|
||
@Override | ||
protected void doHealthCheck(Health.Builder builder) { | ||
try { | ||
s3Client.headBucket(HeadBucketRequest.builder().bucket(bucketName).build()); | ||
builder.up().withDetails(Map.of("info", "S3 Bucket Available")).build(); | ||
} catch (Exception e) { | ||
LOGGER.warn("S3 health check failed to get info of bucket {}", bucketName, e); | ||
builder | ||
.down(e) | ||
.withDetails(Map.of("error", "S3 Bucket Not Available: " + e.getMessage())) | ||
.build(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
org.sdase.commons.spring.boot.s3.config.S3Configuration | ||
org.sdase.commons.spring.boot.s3.config.S3Configuration | ||
org.sdase.commons.spring.boot.s3.config.S3HealthIndicator |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
/* | ||
* Copyright 2022- SDA SE Open Industry Solutions (https://www.sda.se) | ||
* | ||
* Use of this source code is governed by an MIT-style | ||
* license that can be found in the LICENSE file or at | ||
* https://opensource.org/licenses/MIT. | ||
*/ | ||
package org.sdase.commons.spring.boot.s3.config; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.mockito.ArgumentMatchers.any; | ||
import static org.mockito.Mockito.doThrow; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junitpioneer.jupiter.SetSystemProperty; | ||
import org.sdase.commons.spring.boot.s3.S3TestApp; | ||
import org.sdase.commons.spring.boot.web.testing.s3.S3Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.context.SpringBootTest; | ||
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; | ||
import org.springframework.boot.test.mock.mockito.SpyBean; | ||
import org.springframework.boot.test.web.client.TestRestTemplate; | ||
import org.springframework.boot.test.web.server.LocalManagementPort; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.test.annotation.DirtiesContext; | ||
import software.amazon.awssdk.services.s3.S3Client; | ||
import software.amazon.awssdk.services.s3.model.HeadBucketRequest; | ||
|
||
@SetSystemProperty(key = "management.health.s3.enabled", value = "true") | ||
@SpringBootTest( | ||
classes = S3TestApp.class, | ||
webEnvironment = WebEnvironment.RANDOM_PORT, | ||
properties = {"auth.disable=true", "opa.disable=true", "management.server.port=8071"}) | ||
@S3Test | ||
@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Can the dirty context be avoided if we don't use SetSystemProperty? Afaik, this slows down the test suite. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that I want to test when it is enabled and when it is not, so I need to run the server again for each test. In order to do that I either use this approach or I separate in two test classes, one with the health check enabled, and the other one disabled. |
||
class S3HealthIndicatorIntegrationTest { | ||
|
||
@LocalManagementPort private int managementPort; | ||
|
||
@Autowired private TestRestTemplate client; | ||
|
||
@SpyBean private S3Client s3Client; | ||
|
||
@Test | ||
void checkThatS3HealthCheckIsEnabledAndUp() { | ||
var response = getHealthCheckInfoData(); | ||
var s3Info = response.getBody().components().s3(); | ||
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); | ||
assertThat(response.getBody().status()).isEqualTo("UP"); | ||
assertThat(s3Info.status()).isEqualTo("UP"); | ||
assertThat(s3Info.details().info()).isEqualTo("S3 Bucket Available"); | ||
} | ||
|
||
@Test | ||
void checkThatS3HealthCheckIsEnabledAndDown() { | ||
doThrow(new RuntimeException("Simulate s3 health check issue")) | ||
.when(s3Client) | ||
.headBucket(any(HeadBucketRequest.class)); | ||
var response = getHealthCheckInfoData(); | ||
var s3Info = response.getBody().components().s3(); | ||
assertThat(response.getStatusCode().is5xxServerError()).isTrue(); | ||
assertThat(response.getBody().status()).isEqualTo("DOWN"); | ||
assertThat(s3Info.status()).isEqualTo("DOWN"); | ||
assertThat(s3Info.details().error()) | ||
.isEqualTo("S3 Bucket Not Available: Simulate s3 health check issue"); | ||
} | ||
|
||
@Test | ||
@SetSystemProperty(key = "management.health.s3.enabled", value = "false") | ||
void checkThatS3HealthCheckIsNotEnabled() { | ||
var response = getHealthCheckInfoData(); | ||
var s3Info = response.getBody().components().s3(); | ||
assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); | ||
assertThat(response.getBody().status()).isEqualTo("UP"); | ||
assertThat(s3Info).isNull(); | ||
} | ||
|
||
private ResponseEntity<HealthInfo> getHealthCheckInfoData() { | ||
return client.getForEntity( | ||
String.format("http://localhost:%d/healthcheck", managementPort), HealthInfo.class); | ||
} | ||
|
||
record HealthInfo(String status, Components components) { | ||
record Components(S3Info s3) { | ||
record S3Info(String status, S3InfoDetails details) { | ||
record S3InfoDetails(String info, String error) {} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: As this is a SpringBootTest, isn't it possible to set it as property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no, because otherrwise it would ovewrite the
@SetSystemProperty
in thecheckThatS3HealthCheckIsNotEnabled
test.