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.

Sign up for GitHub

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

Jump to bottom

feat(c2cuc): add functionality for updating configuration of other components #1577

Draft
wants to merge 3 commits into
base: c2c-update-configuration
Choose a base branch
from
+99 −11
Draft

feat(c2cuc): add functionality for updating configuration of other components #1577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e99602
chore(c2cuc): use snapshot version of aws-iot-device-sdk for new Upda…
aws-kevinrickard Feb 9, 2024
605bfaa
feat(c2cuc): add basic functionality for updating configuration of ot…
aws-kevinrickard Feb 9, 2024
d55536b
fix: normalize component not found error messages for GetConfiguratio…
aws-kevinrickard Feb 9, 2024
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@
<dependency>
<groupId>software.amazon.awssdk.iotdevicesdk</groupId>
<artifactId>aws-iot-device-sdk</artifactId>
<version>1.19.1</version>
<version>1.19.1-C2CUC-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
Expand Down
28 changes: 21 additions & 7 deletions .../java/com/aws/greengrass/builtin/services/configstore/ConfigStoreIPCEventStreamAgent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@

public class ConfigStoreIPCEventStreamAgent {
private static final Logger logger = LogManager.getLogger(ConfigStoreIPCEventStreamAgent.class);
private static final String COMPONENT_NOT_FOUND_ERROR_FORMAT = "Component config not found for component %s";
private static final String KEY_NOT_FOUND_ERROR_MESSAGE = "Key not found";
private static final String SERVICE_NAME = "service-name";
@Getter(AccessLevel.PACKAGE)
Expand Down Expand Up @@ -192,7 +193,7 @@ public GetConfigurationResponse handleRequest(GetConfigurationRequest request) {
Topics serviceTopics = kernel.findServiceTopic(finalServiceName);

if (serviceTopics == null) {
throw new ResourceNotFoundError(KEY_NOT_FOUND_ERROR_MESSAGE);
throw new ResourceNotFoundError(String.format(COMPONENT_NOT_FOUND_ERROR_FORMAT, finalServiceName));
}

Topics configTopics = serviceTopics.findInteriorChild(CONFIGURATION_CONFIG_KEY);
Expand Down Expand Up @@ -262,6 +263,9 @@ public UpdateConfigurationResponse handleRequest(UpdateConfigurationRequest requ
logger.atDebug().kv(SERVICE_NAME, serviceName).log("Config IPC config update request");
validateRequest(request);

String targetServiceName =
request.getComponentName() == null ? serviceName : request.getComponentName();

String[] keyPath = new String[0];
// Keypath is expected to denote the container node
if (request.getKeyPath() != null) {
Expand All @@ -270,7 +274,14 @@ public UpdateConfigurationResponse handleRequest(UpdateConfigurationRequest requ

Object value = request.getValueToMerge();

Topics serviceTopics = kernel.findServiceTopic(serviceName);
logger.atDebug().kv(SERVICE_NAME, serviceName).log(
"Updating configuration for target component {}, key path {}, and value keys [{}]",
targetServiceName,
Arrays.toString(keyPath),
String.join(", ", request.getValueToMerge().keySet())
);

Topics serviceTopics = kernel.findServiceTopic(targetServiceName);
Copy link
Member

@MikeDombo MikeDombo Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

you need to check for authorization before starting to do the lookup and applying changes. Please do that in this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do

Topics configTopics = serviceTopics.lookupTopics(CONFIGURATION_CONFIG_KEY);
Node node = configTopics.findNode(keyPath);
long updateTime = request.getTimestamp().toEpochMilli();
Expand Down Expand Up @@ -327,14 +338,17 @@ private void validateRequest(UpdateConfigurationRequest request) {
+ restrictedConfigurationFields);
}

Topics serviceTopics = kernel.findServiceTopic(serviceName);
if (serviceTopics == null) {
throw new InvalidArgumentsError("Component config not found for component " + serviceName);
final String targetServiceName =
request.getComponentName() == null ? serviceName : request.getComponentName();

Topics targetServiceTopics = kernel.findServiceTopic(targetServiceName);
if (targetServiceTopics == null) {
throw new InvalidArgumentsError(String.format(COMPONENT_NOT_FOUND_ERROR_FORMAT, targetServiceName));
MikeDombo marked this conversation as resolved.
Show resolved Hide resolved
}
Topics configTopics = serviceTopics.lookupTopics(CONFIGURATION_CONFIG_KEY);
Topics configTopics = targetServiceTopics.lookupTopics(CONFIGURATION_CONFIG_KEY);
Node node = configTopics.findNode(keyPath);
if (node != null && !(node instanceof Topic) && !(node instanceof Topics)) {
logger.atError().kv(SERVICE_NAME, serviceName)
logger.atError().kv(SERVICE_NAME, targetServiceName)
.log("Somehow Node has an unknown type {}", node.getClass());
throw new InvalidArgumentsError("Node corresponding to keypath "
+ request.getKeyPath().toString() + " has an unknown type");
Expand Down
80 changes: 77 additions & 3 deletions ...a/com/aws/greengrass/builtin/services/configstore/ConfigStoreIPCEventStreamAgentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@
import static com.aws.greengrass.lifecyclemanager.GreengrassService.SERVICES_NAMESPACE_TOPIC;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -221,7 +223,7 @@ void GIVEN_get_config_request_WHEN_component_requested_does_not_exist_THEN_fail(
request.setKeyPath(Collections.singletonList("AnyKey"));
ResourceNotFoundError error = assertThrows(ResourceNotFoundError.class, () ->
agent.getGetConfigurationHandler(mockContext).handleRequest(request));
assertEquals("Key not found", error.getMessage());
assertEquals("Component config not found for component WrongComponent", error.getMessage());
}

@Test
Expand Down Expand Up @@ -262,7 +264,21 @@ void GIVEN_agent_running_WHEN_update_config_request_for_ACL_THEN_update_fails()
request.setTimestamp(Instant.now());
Exception e = assertThrows(InvalidArgumentsError.class,
() -> agent.getUpdateConfigurationHandler(mockContext).handleRequest(request));
assertTrue(e.getMessage().contains("Config update is not allowed for following fields"));
assertThat(e.getMessage(), containsString("Config update is not allowed for following fields"));
}

@Test
void GIVEN_component_name_provided_WHEN_update_config_request_for_ACL_THEN_update_fails() {
when(mockAuthenticationData.getIdentityLabel()).thenReturn(TEST_COMPONENT_A);
UpdateConfigurationRequest request = new UpdateConfigurationRequest();
request.setComponentName(TEST_COMPONENT_B);
request.setKeyPath(Collections.EMPTY_LIST);
request.setValueToMerge(Collections.singletonMap(ACCESS_CONTROL_NAMESPACE_TOPIC, Collections.singletonMap(
"aws.greengrass.ipc.pubsub", "policy")));
request.setTimestamp(Instant.now());
Exception e = assertThrows(InvalidArgumentsError.class,
() -> agent.getUpdateConfigurationHandler(mockContext).handleRequest(request));
assertThat(e.getMessage(), containsString("Config update is not allowed for following fields"));
}

@Test
Expand All @@ -274,7 +290,7 @@ void GIVEN_agent_running_WHEN_update_config_request_for_ACL_nested_node_THEN_upd
request.setTimestamp(Instant.now());
Exception e = assertThrows(InvalidArgumentsError.class,
() -> agent.getUpdateConfigurationHandler(mockContext).handleRequest(request));
assertTrue(e.getMessage().contains("Config update is not allowed for following fields"));
assertThat(e.getMessage(), containsString("Config update is not allowed for following fields"));
}

@Test
Expand All @@ -293,6 +309,64 @@ void GIVEN_update_config_request_WHEN_update_key_does_not_exist_THEN_create_key(
assertEquals("SomeValue", newConfigKeyTopic.getOnce());
}

@Test
void GIVEN_update_config_request_WHEN_component_requested_does_not_exist_THEN_fail() {
when(mockAuthenticationData.getIdentityLabel()).thenReturn(TEST_COMPONENT_A);
String wrongComponentName = "WrongComponent";

UpdateConfigurationRequest request = new UpdateConfigurationRequest();
request.setComponentName(wrongComponentName);
request.setKeyPath(Collections.singletonList("AnyKeyPath"));
request.setValueToMerge(Collections.singletonMap("AnyKey", "AnyValue"));
request.setTimestamp(Instant.now());
InvalidArgumentsError error = assertThrows(InvalidArgumentsError.class, () ->
agent.getUpdateConfigurationHandler(mockContext).handleRequest(request));
assertEquals("Component config not found for component " + wrongComponentName, error.getMessage());
}

@Test
void GIVEN_component_name_provided_for_update_WHEN_key_path_does_not_exist_THEN_create_key_for_given_component() {
when(kernel.findServiceTopic(TEST_COMPONENT_B))
.thenReturn(configuration.getRoot().lookupTopics(SERVICES_NAMESPACE_TOPIC, TEST_COMPONENT_B));

assertNull(kernel.findServiceTopic(TEST_COMPONENT_B).findTopics(CONFIGURATION_CONFIG_KEY, "NewKey"));

when(mockAuthenticationData.getIdentityLabel()).thenReturn(TEST_COMPONENT_A);
UpdateConfigurationRequest request = new UpdateConfigurationRequest();
request.setKeyPath(Collections.singletonList("NewKey"));
request.setValueToMerge(Collections.singletonMap("NewValueToMergeKey", "SomeValue"));
request.setComponentName(TEST_COMPONENT_B);
request.setTimestamp(Instant.now());
UpdateConfigurationResponse response = agent.getUpdateConfigurationHandler(mockContext).handleRequest(request);
assertNotNull(response);

Topics newKeyTopics = kernel.findServiceTopic(TEST_COMPONENT_B).findTopics(CONFIGURATION_CONFIG_KEY, "NewKey");
assertNotNull(newKeyTopics);
Topic newValueToMergeTopic = kernel.findServiceTopic(TEST_COMPONENT_B).find(CONFIGURATION_CONFIG_KEY, "NewKey", "NewValueToMergeKey");
assertNotNull(newValueToMergeTopic);
assertEquals("SomeValue", newValueToMergeTopic.getOnce());
}

@Test
void GIVEN_component_name_provided_for_update_WHEN_key_not_provided_THEN_merge_values_at_config_root() {
when(kernel.findServiceTopic(TEST_COMPONENT_B))
.thenReturn(configuration.getRoot().lookupTopics(SERVICES_NAMESPACE_TOPIC, TEST_COMPONENT_B));

assertNull(kernel.findServiceTopic(TEST_COMPONENT_B).find(CONFIGURATION_CONFIG_KEY, "NewKey"));

when(mockAuthenticationData.getIdentityLabel()).thenReturn(TEST_COMPONENT_A);
UpdateConfigurationRequest request = new UpdateConfigurationRequest();
request.setValueToMerge(Collections.singletonMap("NewKey", "SomeValue"));
request.setComponentName(TEST_COMPONENT_B);
request.setTimestamp(Instant.now());
UpdateConfigurationResponse response = agent.getUpdateConfigurationHandler(mockContext).handleRequest(request);
assertNotNull(response);

Topic newConfigKeyTopic = kernel.findServiceTopic(TEST_COMPONENT_B).find(CONFIGURATION_CONFIG_KEY, "NewKey");
assertNotNull(newConfigKeyTopic);
assertEquals("SomeValue", newConfigKeyTopic.getOnce());
}

@Test
void GIVEN_update_config_request_WHEN_proposed_timestamp_is_stale_THEN_fail() {
when(mockAuthenticationData.getIdentityLabel()).thenReturn(TEST_COMPONENT_A);
Expand Down
Loading