Skip to content

Commit

Permalink
Refreshing AWS Secret values on demand (#5347)
Browse files Browse the repository at this point in the history
* refreshing the values on demand

Signed-off-by: Santhosh Gandhe <[email protected]>

* refreshing only for the forbidden or unauthorized error casea and additional test cases to cover those scenarios

Signed-off-by: Santhosh Gandhe <[email protected]>

---------

Signed-off-by: Santhosh Gandhe <[email protected]>
  • Loading branch information
san81 authored Jan 21, 2025
1 parent fe8a94b commit 01c9b4a
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public interface PluginConfigVariable {
*/
void setValue(Object updatedValue);

/**
* Refresh the secret value on demand
*/
void refresh();

/**
* Returns if the variable is updatable.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ public void setValue(Object updatedValue) {
this.secretValue = updatedValue.toString();
}

@Override
public void refresh() {
}

@Override
public boolean isUpdatable() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ public void setValue(Object newValue) {
this.secretValue = newValue;
}

@Override
public void refresh() {
secretsSupplier.refresh(secretId);
}


@Override
public boolean isUpdatable() {
return isUpdatable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -81,4 +83,11 @@ void testSetValueSuccess(final String input) {
assertThat(objectUnderTest.getValue(), equalTo(input));
}


@Test
void testRefreshSecretsWithKey() {
objectUnderTest.refresh();
verify(secretsSupplier, times(1)).refresh(secretId);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,16 @@ public void renewCredentials() {
} catch (HttpClientErrorException ex) {
this.expireTime = Instant.ofEpochMilli(0);
this.expiresInSeconds = 0;
HttpStatus statusCode = ex.getStatusCode();
log.error("Failed to renew access token. Status code: {}, Error Message: {}",
ex.getRawStatusCode(), ex.getMessage());
throw new RuntimeException("Failed to renew access token" + ex.getMessage(), ex);
statusCode, ex.getMessage());
if (statusCode == HttpStatus.FORBIDDEN || statusCode == HttpStatus.UNAUTHORIZED) {
log.info("Trying to refresh the secrets");
// Try refreshing the secrets and see if that helps
// Refreshing one of the secret refreshes the entire store so we are good to trigger refresh on just one
jiraSourceConfig.getAuthenticationConfig().getOauth2Config().getAccessToken().refresh();
}
throw new RuntimeException("Failed to renew access token message:" + ex.getMessage(), ex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opensearch.dataprepper.model.plugin.PluginConfigVariable;
import org.opensearch.dataprepper.plugins.source.jira.JiraSourceConfig;
import org.opensearch.dataprepper.plugins.source.jira.configuration.Oauth2Config;
import org.opensearch.dataprepper.plugins.source.jira.exception.UnAuthorizedException;
import org.opensearch.dataprepper.test.helper.ReflectivelySetField;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -52,6 +55,9 @@ public class JiraOauthConfigTest {

JiraSourceConfig jiraSourceConfig;

@Mock
PluginConfigVariable accessTokenVariable;

@BeforeEach
void setUp() {
jiraSourceConfig = createJiraConfigurationFromYaml("oauth2-auth-jira-pipeline.yaml");
Expand Down Expand Up @@ -89,12 +95,31 @@ void testRenewToken() throws InterruptedException {
}

@Test
void testFailedToRenewAccessToken() {
void testFailedToRenewAccessToken() throws NoSuchFieldException, IllegalAccessException {
JiraOauthConfig jiraOauthConfig = new JiraOauthConfig(jiraSourceConfig);
Oauth2Config oauth2Config = jiraSourceConfig.getAuthenticationConfig().getOauth2Config();
ReflectivelySetField.setField(Oauth2Config.class, oauth2Config, "accessToken", accessTokenVariable);
when(restTemplateMock.postForEntity(any(String.class), any(HttpEntity.class), any(Class.class)))
.thenThrow(HttpClientErrorException.class);
jiraOauthConfig.restTemplate = restTemplateMock;
assertThrows(RuntimeException.class, jiraOauthConfig::renewCredentials);
verify(oauth2Config.getAccessToken(), times(0))
.refresh();
}

@Test
void testFailedToRenewAccessToken_with_unauthorized_and_trigger_secrets_refresh()
throws NoSuchFieldException, IllegalAccessException {
JiraOauthConfig jiraOauthConfig = new JiraOauthConfig(jiraSourceConfig);
Oauth2Config oauth2Config = jiraSourceConfig.getAuthenticationConfig().getOauth2Config();
ReflectivelySetField.setField(Oauth2Config.class, oauth2Config, "accessToken", accessTokenVariable);
HttpClientErrorException unAuthorizedException = new HttpClientErrorException(HttpStatus.UNAUTHORIZED);
when(restTemplateMock.postForEntity(any(String.class), any(HttpEntity.class), any(Class.class)))
.thenThrow(unAuthorizedException);
jiraOauthConfig.restTemplate = restTemplateMock;
assertThrows(RuntimeException.class, jiraOauthConfig::renewCredentials);
verify(oauth2Config.getAccessToken(), times(1))
.refresh();
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public void setValue(Object someValue) {
this.defaultValue = someValue;
}

@Override
public void refresh() {
}

@Override
public boolean isUpdatable() {
return true;
Expand Down

0 comments on commit 01c9b4a

Please sign in to comment.