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

Update renewed Jira refresh token back to the secrets store #5324

Merged
merged 19 commits into from
Jan 17, 2025
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aa7ca59
Introduced PluginConfigVariable interaface to provide ability for the…
san81 Jan 13, 2025
0c34119
renewed access token and refresh tokens are now updated back in the s…
san81 Jan 13, 2025
cc82281
better naming
san81 Jan 13, 2025
2f640df
Merge branch 'opensearch-project:main' into secrets-variable-interface
san81 Jan 13, 2025
64bf7bc
fixing the test cases based on the new PluginConfigVariable attribute…
san81 Jan 13, 2025
4df4fba
improving the coverage
san81 Jan 13, 2025
a384d85
Keeping the existing values in the secret. Just updating an existing key
san81 Jan 14, 2025
2f26378
Merge branch 'opensearch-project:main' into secrets-variable-interface
san81 Jan 15, 2025
ca1ec88
Allowing secrets manager update without a key and also some additiona…
san81 Jan 15, 2025
2f98ebc
isUpdatable boolean is introduced and its corresponding tests
san81 Jan 15, 2025
f8802e5
additional coverage
san81 Jan 16, 2025
eb2e431
implementing newly added method
san81 Jan 16, 2025
76a136a
switching PluginConfigVariable from refreshToken to accessToken
san81 Jan 16, 2025
188264e
Only the master node is responsible for Token refresh
san81 Jan 16, 2025
009eafe
Added addition parameter in the API to accept the secrets version to …
san81 Jan 16, 2025
78de3aa
better naming
san81 Jan 16, 2025
7b15c95
removing setting a versionId for idempotency
san81 Jan 16, 2025
1d14234
removed constructor argument to PluginConfigVariable
san81 Jan 16, 2025
8b946af
Merge branch 'opensearch-project:main' into secrets-variable-interface
san81 Jan 17, 2025
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
Prev Previous commit
Next Next commit
Allowing secrets manager update without a key and also some additiona…
…l test coverage

Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
san81 committed Jan 15, 2025
commit ca1ec883e283945e90f29aab7f1e3622af87c6a7
Original file line number Diff line number Diff line change
@@ -13,17 +13,21 @@
* Interface for a Extension Plugin configuration variable.
* It gives access to the details of a defined extension variable.
*
* @since 1.2
* @since 2.11
*/
public interface PluginConfigVariable {

/**
* Returns the value of this variable.
*
* @return the value of this variable
*/
Object getValue();

/**
* If this variable is updatable, this method helps to set a new value for this variable
*
* @param updatedValue the new value to set
*/
void setValue(Object someValue);
void setValue(Object updatedValue);
}
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opensearch.dataprepper.model.plugin.PluginConfigValueTranslator;
import org.opensearch.dataprepper.model.plugin.PluginConfigVariable;

import java.io.IOException;
import java.math.BigDecimal;
@@ -30,6 +31,8 @@

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;

@@ -43,6 +46,32 @@ class VariableExpanderTest {

private VariableExpander objectUnderTest;

private static Stream<Arguments> getNonStringTypeArguments() {
return Stream.of(Arguments.of(Boolean.class, "true", true),
Arguments.of(Short.class, "2", (short) 2),
Arguments.of(Integer.class, "10", 10),
Arguments.of(Long.class, "200", 200L),
Arguments.of(Double.class, "1.23", 1.23d),
Arguments.of(Float.class, "2.15", 2.15f),
Arguments.of(BigDecimal.class, "2.15", BigDecimal.valueOf(2.15)),
Arguments.of(Map.class, "{}", Collections.emptyMap()));
}

private static Stream<Arguments> getStringTypeArguments() {
final String testRandomValue = "non-secret-prefix-" + RandomStringUtils.randomAlphabetic(5);
return Stream.of(Arguments.of(String.class, String.format("\"%s\"", testRandomValue),
testRandomValue),
Arguments.of(Duration.class, "\"PT15M\"", Duration.parse("PT15M")),
Arguments.of(Boolean.class, "\"true\"", true),
Arguments.of(Short.class, "\"2\"", (short) 2),
Arguments.of(Integer.class, "\"10\"", 10),
Arguments.of(Long.class, "\"200\"", 200L),
Arguments.of(Double.class, "\"1.23\"", 1.23d),
Arguments.of(Float.class, "\"2.15\"", 2.15f),
Arguments.of(BigDecimal.class, "\"2.15\"", BigDecimal.valueOf(2.15)),
Arguments.of(Character.class, "\"c\"", 'c'));
}

@BeforeEach
void setUp() {
objectUnderTest = new VariableExpander(OBJECT_MAPPER, Set.of(pluginConfigValueTranslator));
@@ -107,29 +136,48 @@ void testTranslateJsonParserWithStringValue_translate_success(
assertThat(actualResult, equalTo(expectedResult));
}

private static Stream<Arguments> getNonStringTypeArguments() {
return Stream.of(Arguments.of(Boolean.class, "true", true),
Arguments.of(Short.class, "2", (short) 2),
Arguments.of(Integer.class, "10", 10),
Arguments.of(Long.class, "200", 200L),
Arguments.of(Double.class, "1.23", 1.23d),
Arguments.of(Float.class, "2.15", 2.15f),
Arguments.of(BigDecimal.class, "2.15", BigDecimal.valueOf(2.15)),
Arguments.of(Map.class, "{}", Collections.emptyMap()));
@Test
void testTranslateJsonParserWithSPluginConfigVariableValue_translate_success() throws IOException {
final String testSecretKey = "testSecretKey";
final String testTranslatorKey = "test_prefix";
final String testSecretReference = String.format("${{%s:%s}}", testTranslatorKey, testSecretKey);
final JsonParser jsonParser = JSON_FACTORY.createParser(String.format("\"%s\"", testSecretReference));
jsonParser.nextToken();
PluginConfigVariable mockPluginConfigVariable = new PluginConfigVariable() {

String secretValue = "samplePluginConfigValue";

@Override
public Object getValue() {
return secretValue;
}

@Override
public void setValue(Object updatedValue) {
this.secretValue = updatedValue.toString();
}
};
when(pluginConfigValueTranslator.getPrefix()).thenReturn(testTranslatorKey);
when(pluginConfigValueTranslator.translateToPluginConfigVariable(eq(testSecretKey)))
.thenReturn(mockPluginConfigVariable);
objectUnderTest = new VariableExpander(OBJECT_MAPPER, Set.of(pluginConfigValueTranslator));
final Object actualResult = objectUnderTest.translate(jsonParser, PluginConfigVariable.class);
assertNotNull(actualResult);
assertThat(actualResult, equalTo(mockPluginConfigVariable));
}

private static Stream<Arguments> getStringTypeArguments() {
final String testRandomValue = "non-secret-prefix-" + RandomStringUtils.randomAlphabetic(5);
return Stream.of(Arguments.of(String.class, String.format("\"%s\"", testRandomValue),
testRandomValue),
Arguments.of(Duration.class, "\"PT15M\"", Duration.parse("PT15M")),
Arguments.of(Boolean.class, "\"true\"", true),
Arguments.of(Short.class, "\"2\"", (short) 2),
Arguments.of(Integer.class, "\"10\"", 10),
Arguments.of(Long.class, "\"200\"", 200L),
Arguments.of(Double.class, "\"1.23\"", 1.23d),
Arguments.of(Float.class, "\"2.15\"", 2.15f),
Arguments.of(BigDecimal.class, "\"2.15\"", BigDecimal.valueOf(2.15)),
Arguments.of(Character.class, "\"c\"", 'c'));
@Test
void testTranslateJsonParserWithSPluginConfigVariableValue_translate_failure() throws IOException {
final String testSecretKey = "testSecretKey";
final String testTranslatorKey = "test_prefix";
final String testSecretReference = String.format("${{%s:%s}}", testTranslatorKey, testSecretKey);
final JsonParser jsonParser = JSON_FACTORY.createParser(String.format("\"%s\"", testSecretReference));
jsonParser.nextToken();
when(pluginConfigValueTranslator.getPrefix()).thenReturn(testTranslatorKey);
when(pluginConfigValueTranslator.translateToPluginConfigVariable(eq(testSecretKey)))
.thenThrow(IllegalArgumentException.class);
objectUnderTest = new VariableExpander(OBJECT_MAPPER, Set.of(pluginConfigValueTranslator));
assertThrows(IllegalArgumentException.class,
() -> objectUnderTest.translate(jsonParser, PluginConfigVariable.class));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.dataprepper.plugins.aws;
@@ -129,10 +135,27 @@ private Object retrieveSecretsFromSecretManager(final AwsSecretManagerConfigurat
}
}

@Override
public String updateValue(String secretId, Object newValue) {
return updateValue(secretId, null, newValue);
}

@Override
public String updateValue(String secretId, String keyToUpdate, Object newValue) {
final Map<String, Object> keyValuePairs = (Map<String, Object>) secretIdToValue.get(secretId);
keyValuePairs.put(keyToUpdate, newValue);
Object currentSecretStore = secretIdToValue.get(secretId);
if (currentSecretStore instanceof Map) {
if (keyToUpdate == null) {
throw new IllegalArgumentException(
String.format("Key to update cannot be null for a key value based secret. secretId: %s", secretId));
}
final Map<String, Object> keyValuePairs = (Map<String, Object>) currentSecretStore;
keyValuePairs.put(keyToUpdate, newValue);
} else {
//This store is not a key value pair store. It is just a value store.
//If we are here, either KeyToUpdate passed is null or we simply ignore it and just put value in the store
secretIdToValue.put(secretId, newValue);
}
// assuming all the secrets are string based (not binary)
String secretKeyValueMapAsString = (String) retrieveValue(secretId);
AwsSecretManagerConfiguration awsSecretManagerConfiguration = awsSecretManagerConfigurationMap.get(secretId);
PutSecretValueRequest putSecretValueRequest =
Original file line number Diff line number Diff line change
@@ -22,4 +22,14 @@ public interface SecretsSupplier {
* @return The version id of the secret after the update
*/
String updateValue(String secretId, String keyToUpdate, Object newValueToSet);

/**
* Update the value of secret store (which is not a key value secret store) and responds
* with the version id of the secret after the update.
*
* @param secretId The id of the secret to be updated
* @param newValueToSet The value of the secret to be updated
* @return The version id of the secret after the update
*/
String updateValue(String secretId, Object newValueToSet);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.dataprepper.plugins.aws;
@@ -138,8 +144,10 @@ void testCreateGetSecretValueRequest() throws IOException {
verify(getSecretValueRequestBuilder).secretId("test-secret");
}

@Test
void testCreatePutSecretValueRequest() throws IOException {

@ParameterizedTest
@ValueSource(strings = {"", " ", "secretValue", "{\"keyToUpdate\", \"newValue\"}"})
void testPutSecretValueRequest_construct_put_request(String secretValueToStore) throws IOException {
when(putSecretValueRequestBuilder.secretId(anyString())).thenReturn(putSecretValueRequestBuilder);
when(putSecretValueRequestBuilder.secretString(anyString())).thenReturn(putSecretValueRequestBuilder);
when(putSecretValueRequestBuilder.build()).thenReturn(putSecretValueRequest);
@@ -151,7 +159,8 @@ void testCreatePutSecretValueRequest() throws IOException {
mockStatic(PutSecretValueRequest.class)) {
putSecretValueRequestMockedStatic.when(PutSecretValueRequest::builder).thenReturn(
putSecretValueRequestBuilder);
assertThat(awsSecretManagerConfiguration.putSecretValueRequest("{\"keyToUpdate\", \"newValue\"}"), is(putSecretValueRequest));
assertThat(awsSecretManagerConfiguration.putSecretValueRequest(secretValueToStore),
is(putSecretValueRequest));
}
verify(putSecretValueRequestBuilder).secretId("test-secret");
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.dataprepper.plugins.aws;
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.dataprepper.plugins.aws;
@@ -27,6 +33,7 @@

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@@ -173,19 +180,45 @@ void testRefreshSecretsWithoutKey() {
assertThat(objectUnderTest.retrieveValue(TEST_AWS_SECRET_CONFIGURATION_NAME), equalTo(newTestValue));
}

@Test
void testUpdateValue() {
@ParameterizedTest
@ValueSource(strings = {"", " ", "newValue", "{\"key\":\"oldValue\"}", "{\"a\":\"b\"}"})
void testUpdateValue(String valueToSet) {
when(awsSecretManagerConfiguration.putSecretValueRequest(any())).thenReturn(putSecretValueRequest);
when(secretsManagerClient.putSecretValue(eq(putSecretValueRequest))).thenReturn(putSecretValueResponse);
final String testValue = "{\"key\":\"oldValue\"}";
when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(testValue);
String newVersionId = UUID.randomUUID().toString();
when(putSecretValueResponse.versionId()).thenReturn(newVersionId);
objectUnderTest = new AwsSecretsSupplier(secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER);
assertThat(objectUnderTest.updateValue(TEST_AWS_SECRET_CONFIGURATION_NAME, "key", "newValue"),
assertThat(objectUnderTest.updateValue(TEST_AWS_SECRET_CONFIGURATION_NAME, "key", valueToSet),
equalTo(newVersionId));
}

@Test
void testUpdateValue_null_key_throws_exception() {
when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse);
objectUnderTest = new AwsSecretsSupplier(secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER);
assertThrows(IllegalArgumentException.class,
() -> objectUnderTest.updateValue(TEST_AWS_SECRET_CONFIGURATION_NAME, null, "newValue"));
}

@ParameterizedTest
@ValueSource(strings = {"", " ", "newValue"})
void testUpdateValue_null_key_doesnot_throws_exception_when_value_is_not_key_value_pair(String secretValueToSet) {
when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest);
when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn(
Map.of(TEST_AWS_SECRET_CONFIGURATION_NAME, awsSecretManagerConfiguration)
);
when(awsSecretManagerConfiguration.createSecretManagerClient()).thenReturn(secretsManagerClient);
when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(TEST_VALUE);
when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse);
when(awsSecretManagerConfiguration.putSecretValueRequest(any())).thenReturn(putSecretValueRequest);
when(secretsManagerClient.putSecretValue(eq(putSecretValueRequest))).thenReturn(putSecretValueResponse);
String versionId = UUID.randomUUID().toString();
when(putSecretValueResponse.versionId()).thenReturn(versionId);
objectUnderTest = new AwsSecretsSupplier(secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER);
String newValue = objectUnderTest.updateValue(TEST_AWS_SECRET_CONFIGURATION_NAME, secretValueToSet);
assertEquals(versionId, newValue);
}

@Test
void testUpdateValueFailed() {
when(awsSecretManagerConfiguration.putSecretValueRequest(any())).thenReturn(putSecretValueRequest);