Skip to content

Commit b7feca4

Browse files
yitingbyitingb
authored
Merge branch 'main' into update-ubuntu
2 parents ec05dfc + 2870f12 commit b7feca4

7 files changed

Lines changed: 107 additions & 45 deletions

File tree

‎src/integrationtests/java/com/aws/greengrass/integrationtests/e2e/tes/TESTest.java‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import com.aws.greengrass.logging.api.Logger;
2121
import com.aws.greengrass.logging.impl.LogManager;
2222
import com.aws.greengrass.tes.CredentialRequestHandler;
23+
import com.aws.greengrass.tes.HttpServerImpl;
2324
import com.aws.greengrass.tes.TokenExchangeService;
25+
import com.aws.greengrass.util.Coerce;
2426
import com.aws.greengrass.util.IamSdkClientFactory;
2527
import com.aws.greengrass.util.IotSdkClientFactory;
2628
import org.junit.jupiter.api.AfterAll;
@@ -43,16 +45,19 @@
4345
import java.nio.charset.StandardCharsets;
4446
import java.nio.file.Path;
4547
import java.util.Collections;
48+
import java.util.Objects;
4649
import java.util.UUID;
4750
import java.util.concurrent.CountDownLatch;
4851
import java.util.concurrent.TimeUnit;
52+
import java.util.concurrent.atomic.AtomicReference;
4953

5054
import static com.aws.greengrass.componentmanager.KernelConfigResolver.CONFIGURATION_CONFIG_KEY;
55+
import static com.aws.greengrass.deployment.DeviceConfiguration.IOT_ROLE_ALIAS_TOPIC;
5156
import static com.aws.greengrass.easysetup.DeviceProvisioningHelper.ThingInfo;
5257
import static com.aws.greengrass.integrationtests.e2e.BaseE2ETestCase.E2ETEST_ENV_STAGE;
5358
import static com.aws.greengrass.lifecyclemanager.GreengrassService.SERVICES_NAMESPACE_TOPIC;
5459
import static com.aws.greengrass.lifecyclemanager.GreengrassService.SETENV_CONFIG_NAMESPACE;
55-
import static com.aws.greengrass.deployment.DeviceConfiguration.IOT_ROLE_ALIAS_TOPIC;
60+
import static com.aws.greengrass.tes.TokenExchangeService.PORT_TOPIC;
5661
import static com.aws.greengrass.tes.TokenExchangeService.TES_URI_ENV_VARIABLE_NAME;
5762
import static com.aws.greengrass.tes.TokenExchangeService.TOKEN_EXCHANGE_SERVICE_TOPICS;
5863
import static com.aws.greengrass.testcommons.testutilities.ExceptionLogProtector.ignoreExceptionUltimateCauseOfType;
@@ -131,6 +136,43 @@ static void tearDown() throws URISyntaxException {
131136
}
132137
}
133138

139+
@Test
140+
void GIVEN_iot_role_alias_WHEN_port_changes_THEN_valid_credentials_are_returned_from_new_port() throws Exception {
141+
// Get current port and calculate new port
142+
int currentPort = Coerce.toInt(
143+
kernel.getConfig().lookupTopics(SERVICES_NAMESPACE_TOPIC, TOKEN_EXCHANGE_SERVICE_TOPICS)
144+
.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC));
145+
int newPort = currentPort + 1;
146+
String expectedUri = String.format("http://localhost:%d%s", newPort, HttpServerImpl.URL);
147+
148+
// Setup server restart detection
149+
CountDownLatch serverRestarted = new CountDownLatch(1);
150+
AtomicReference<String> urlString = new AtomicReference<>();
151+
kernel.getConfig().find(SETENV_CONFIG_NAMESPACE, TES_URI_ENV_VARIABLE_NAME).subscribe((why, newv) -> {
152+
urlString.set(Coerce.toString(newv));
153+
if (urlString.get().equals(expectedUri)) {
154+
serverRestarted.countDown();
155+
}
156+
});
157+
158+
// Change port and wait for server restart
159+
kernel.getConfig().lookupTopics(SERVICES_NAMESPACE_TOPIC, TOKEN_EXCHANGE_SERVICE_TOPICS)
160+
.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC).withValue(newPort);
161+
assertTrue(serverRestarted.await(5, TimeUnit.SECONDS), "Server did not restart within 5 seconds");
162+
assertEquals(expectedUri, urlString.get(), "New URL does not match expected URI");
163+
164+
// Get authentication token and make request
165+
String token = Objects.requireNonNull(kernel.getConfig()
166+
.findTopics(SERVICES_NAMESPACE_TOPIC, AuthenticationHandler.AUTHENTICATION_TOKEN_LOOKUP_KEY)).iterator()
167+
.next().getName();
168+
169+
String response = getResponseString(new URL(urlString.get()), token);
170+
171+
// Verify response format
172+
assertThat(response, matchesPattern(
173+
"\\{\"AccessKeyId\":\".+\",\"SecretAccessKey\":\".+\",\"Expiration\":\".+\",\"Token\":\".+\"\\}"));
174+
}
175+
134176
@Test
135177
void GIVEN_iot_role_alias_WHEN_tes_is_queried_THEN_valid_credentials_are_returned(ExtensionContext context)
136178
throws Exception {

‎src/main/java/com/aws/greengrass/dependency/Context.java‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ private <T> Value<T> getValue(Class<T> clazz, Object tag) {
121121
}
122122

123123
public <T> T newInstance(Class<T> cl) {
124-
return new Value<>(cl, null).get();
124+
return new Value<>(cl, null).getObjectWithoutInjection();
125125
}
126126

127127
/**
@@ -434,6 +434,17 @@ public final T get() {
434434
return constructObjectWithInjection();
435435
}
436436

437+
/**
438+
* Constructs an object without injection.
439+
* @return object
440+
*/
441+
public final T getObjectWithoutInjection() {
442+
if (object != null && injectionCompleted) {
443+
return object;
444+
}
445+
return constructObject();
446+
}
447+
437448

438449
/**
439450
* Put a new object instance and inject fields with pre and post actions, if the new object is not equal to
@@ -462,7 +473,7 @@ final T putAndInjectFields(T newObject) {
462473
}
463474

464475
@SuppressWarnings("PMD.AvoidCatchingThrowable")
465-
private T constructObjectWithInjection() {
476+
private T constructObject() {
466477
try (LockScope ls = LockScope.lock(lock)) {
467478
if (object != null) {
468479
return object;
@@ -483,17 +494,23 @@ private T constructObjectWithInjection() {
483494
int paramCount = pickedConstructor.getParameterCount();
484495
if (paramCount == 0) {
485496
// no arg constructor
486-
return putAndInjectFields(pickedConstructor.newInstance());
497+
return pickedConstructor.newInstance();
487498
}
488499

489500
Object[] args = getOrCreateArgInstances(clazz, pickedConstructor, paramCount);
490-
return putAndInjectFields(pickedConstructor.newInstance(args));
501+
return pickedConstructor.newInstance(args);
491502
} catch (Throwable ex) {
492503
throw new IllegalArgumentException("Can't create instance of " + targetClass.getName(), ex);
493504
}
494505
}
495506
}
496507

508+
private T constructObjectWithInjection() {
509+
try (LockScope ls = LockScope.lock(lock)) {
510+
return putAndInjectFields(constructObject());
511+
}
512+
}
513+
497514
private Object[] getOrCreateArgInstances(Class<T> clazz, Constructor<T> pickedConstructor, int argCount) {
498515
Object[] args = new Object[argCount];
499516
Class[] argTypes = pickedConstructor.getParameterTypes();
@@ -549,8 +566,9 @@ private Constructor<T> pickConstructor(Class<T> clazz) throws NoSuchMethodExcept
549566
}
550567

551568
/**
552-
* Computes and return T if object instance is null. TODO revisit to see if there is a better way because the
553-
* mapping function usage is weird.
569+
* Computes and returns T if object instance is null. The mapping function used for the instance creation should
570+
* not inject the created object into the context as it will anyway be injected as part of this method.
571+
* TODO revisit to see if there is a better way because the mapping function usage is weird.
554572
*
555573
* @param mappingFunction maps from Value to T
556574
* @param <E> CheckedException
@@ -563,7 +581,6 @@ public final <E extends Exception> T computeObjectIfEmpty(
563581
if (object != null) {
564582
return object;
565583
}
566-
567584
return putAndInjectFields(mappingFunction.apply(this));
568585
}
569586
}

‎src/main/java/com/aws/greengrass/lifecyclemanager/Kernel.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ public Topics findServiceTopic(String serviceName) {
444444
*/
445445
public GreengrassService locate(String name) throws ServiceLoadException {
446446
return context.getValue(GreengrassService.class, name).computeObjectIfEmpty(v ->
447-
locateGreengrassService(v, name, this::locate));
447+
createGreengrassServiceInstance(v, name, this::locate));
448448
}
449449

450450
/**
@@ -456,7 +456,7 @@ public GreengrassService locate(String name) throws ServiceLoadException {
456456
public GreengrassService locateIgnoreError(String name) {
457457
return context.getValue(GreengrassService.class, name).computeObjectIfEmpty(v -> {
458458
try {
459-
return locateGreengrassService(v, name, this::locateIgnoreError);
459+
return createGreengrassServiceInstance(v, name, this::locateIgnoreError);
460460
} catch (ServiceLoadException e) {
461461
logger.atError().log("Cannot load service", e);
462462
return new UnloadableService(
@@ -481,7 +481,7 @@ private void executeBootstrapTasksAndShutdown(BootstrapManager bootstrapManager,
481481

482482
@SuppressWarnings(
483483
{"UseSpecificCatch", "PMD.AvoidCatchingThrowable", "PMD.AvoidDeeplyNestedIfStmts", "PMD.ConfusingTernary"})
484-
private GreengrassService locateGreengrassService(Context.Value v, String name, CrashableFunction<String,
484+
private GreengrassService createGreengrassServiceInstance(Context.Value v, String name, CrashableFunction<String,
485485
GreengrassService, ServiceLoadException> locateFunction) throws ServiceLoadException {
486486
Topics serviceRootTopics = findServiceTopic(name);
487487

‎src/main/java/com/aws/greengrass/tes/TokenExchangeService.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
2020

2121
import java.io.IOException;
22-
import java.util.Arrays;
22+
import java.util.Collections;
2323
import java.util.HashSet;
2424
import javax.inject.Inject;
2525

@@ -56,10 +56,19 @@ public TokenExchangeService(Topics topics,
5656
CredentialRequestHandler credentialRequestHandler,
5757
AuthorizationHandler authZHandler, DeviceConfiguration deviceConfiguration) {
5858
super(topics);
59-
// Port change should not be allowed
60-
topics.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC).dflt(DEFAULT_PORT)
61-
.subscribe((why, newv) -> port = Coerce.toInt(newv));
62-
59+
port = Coerce.toInt(config.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC).dflt(DEFAULT_PORT));
60+
config.subscribe((why, node) -> {
61+
if (node != null && node.childOf(PORT_TOPIC)) {
62+
logger.atDebug("tes-config-change").kv("node", node).kv("why", why).log();
63+
port = Coerce.toInt(node);
64+
Topic activePortTopic = config.lookup(CONFIGURATION_CONFIG_KEY, ACTIVE_PORT_TOPIC);
65+
if (port != Coerce.toInt(activePortTopic)) {
66+
logger.atInfo("tes-config-change").kv(PORT_TOPIC, port).kv("node", node).kv("why", why)
67+
.log("Restarting TES server due to port config change");
68+
requestRestart();
69+
}
70+
}
71+
});
6372
deviceConfiguration.getIotRoleAlias().subscribe((why, newv) -> {
6473
iotRoleAlias = Coerce.toString(newv);
6574
});
@@ -72,7 +81,8 @@ public TokenExchangeService(Topics topics,
7281
public void postInject() {
7382
super.postInject();
7483
try {
75-
authZHandler.registerComponent(this.getName(), new HashSet<>(Arrays.asList(AUTHZ_TES_OPERATION)));
84+
authZHandler.registerComponent(this.getName(),
85+
new HashSet<>(Collections.singletonList(AUTHZ_TES_OPERATION)));
7686
} catch (AuthorizationException e) {
7787
serviceErrored(e);
7888
}

‎src/main/java/com/aws/greengrass/util/platforms/unix/UnixExec.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,20 +123,21 @@ public void close() throws IOException {
123123
for (PidProcess pp : pidProcesses) {
124124
pp.waitFor(gracefulShutdownTimeout.getSeconds(), TimeUnit.SECONDS);
125125
}
126-
if (pidProcesses.stream().anyMatch(pidProcess -> {
126+
boolean isAnyProcessAlive = pidProcesses.stream().anyMatch(pidProcess -> {
127127
try {
128128
return pidProcess.isAlive();
129129
} catch (IOException ignored) {
130130
} catch (InterruptedException e) {
131131
Thread.currentThread().interrupt();
132132
}
133133
return false;
134-
})) {
134+
});
135+
if (isAnyProcessAlive) {
135136
logger.atWarn()
136137
.log("Command {} did not respond to interruption within timeout. Going to kill it now",
137138
this);
139+
platformInstance.killProcessAndChildren(p, true, pids, userDecorator);
138140
}
139-
platformInstance.killProcessAndChildren(p, true, pids, userDecorator);
140141
if (!p.waitFor(5, TimeUnit.SECONDS) && !isClosed.get()) {
141142
throw new IOException("Could not stop " + this);
142143
}

‎src/main/java/com/aws/greengrass/util/platforms/unix/UnixPlatform.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,13 @@ public UnixUserAttributes lookupCurrentUser() throws IOException {
275275
public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<Integer> additionalPids,
276276
UserDecorator decorator)
277277
throws IOException, InterruptedException {
278-
PidProcess pp = Processes.newPidProcess(process);
278+
PidProcess parentPidProcess = Processes.newPidProcess(process);
279279

280-
logger.atInfo().log("Killing child processes of pid {}, force is {}", pp.getPid(), force);
280+
logger.atInfo().log("Killing child processes of pid {}, force is {}", parentPidProcess.getPid(), force);
281281
Set<Integer> pids;
282282
try {
283283
pids = getChildPids(process);
284-
logger.atDebug().log("Found children of {}. {}", pp.getPid(), pids);
284+
logger.atDebug().log("Found children of {}. {}", parentPidProcess.getPid(), pids);
285285
if (additionalPids != null) {
286286
pids.addAll(additionalPids);
287287
}
@@ -293,6 +293,9 @@ public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<I
293293

294294
killProcess(force, decorator, pid);
295295
}
296+
297+
logger.atInfo().log("Killing parent process {}, force is {}", parentPidProcess.getPid(), force);
298+
killProcess(force, decorator, parentPidProcess.getPid());
296299
} finally {
297300
// calling process.destroy() here when force==false will cause the child process (component process) to be
298301
// terminated immediately. This prevents the component process from shutting down gracefully.
@@ -301,11 +304,13 @@ public Set<Integer> killProcessAndChildren(Process process, boolean force, Set<I
301304
if (process.isAlive()) {
302305
// Kill parent process using privileged user since the parent process might be sudo which a
303306
// non-privileged user can't kill
304-
killProcess(true, getUserDecorator().withUser(getPrivilegedUser()), pp.getPid());
307+
killProcess(true, getUserDecorator().withUser(getPrivilegedUser()), parentPidProcess.getPid());
305308
}
306309
}
307310
}
308311

312+
// add parent pid so caller can validate if every process is properly terminated
313+
pids.add(parentPidProcess.getPid());
309314
return pids;
310315
}
311316

‎src/test/java/com/aws/greengrass/tes/TokenExchangeServiceTest.java‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.amazon.aws.iot.greengrass.component.common.ComponentType;
99
import com.aws.greengrass.authorization.AuthorizationHandler;
1010
import com.aws.greengrass.authorization.exceptions.AuthorizationException;
11+
import com.aws.greengrass.config.ChildChanged;
1112
import com.aws.greengrass.config.Configuration;
1213
import com.aws.greengrass.config.Subscriber;
1314
import com.aws.greengrass.config.Topic;
@@ -131,14 +132,7 @@ void cleanup() throws Exception {
131132
@ParameterizedTest
132133
@ValueSource(ints = {0, 3000})
133134
void GIVEN_token_exchange_service_WHEN_started_THEN_correct_env_set(int port) throws Exception {
134-
Topic portTopic = mock(Topic.class);
135-
when(portTopic.dflt(anyInt())).thenReturn(portTopic);
136-
when(portTopic.subscribe(any())).thenAnswer((a) -> {
137-
((Subscriber) a.getArgument(0)).published(WhatHappened.initialized, portTopic);
138-
return null;
139-
});
140-
when(portTopic.getOnce()).thenReturn(port);
141-
135+
Topic portTopic = Topic.of(new Context(), PORT_TOPIC, port);
142136
Topic roleTopic = mock(Topic.class);
143137
when(roleTopic.subscribe(any())).thenAnswer((a) -> {
144138
((Subscriber) a.getArgument(0)).published(WhatHappened.initialized, roleTopic);
@@ -157,6 +151,11 @@ void GIVEN_token_exchange_service_WHEN_started_THEN_correct_env_set(int port) th
157151
when(configuration.lookup(SERVICES_NAMESPACE_TOPIC, DEFAULT_NUCLEUS_COMPONENT_NAME, CONFIGURATION_CONFIG_KEY,
158152
IOT_ROLE_ALIAS_TOPIC)).thenReturn(roleTopic);
159153

154+
when(config.subscribe(any())).thenAnswer((a) -> {
155+
((ChildChanged) a.getArgument(0)).childChanged(WhatHappened.initialized, portTopic);
156+
return null;
157+
});
158+
160159
TokenExchangeService tes = new TokenExchangeService(config,
161160
mockCredentialHandler,
162161
mockAuthZHandler, deviceConfigurationWithRoleAlias(MOCK_ROLE_ALIAS));
@@ -180,7 +179,6 @@ void GIVEN_token_exchange_service_WHEN_started_THEN_correct_env_set(int port) th
180179
verify(mockAuthZHandler).registerComponent(stringArgumentCaptor.capture(), operationsCaptor.capture());
181180
assertEquals(TOKEN_EXCHANGE_SERVICE_TOPICS, stringArgumentCaptor.getValue());
182181
assertTrue(operationsCaptor.getValue().contains(TokenExchangeService.AUTHZ_TES_OPERATION));
183-
184182
}
185183

186184
@ParameterizedTest
@@ -202,13 +200,7 @@ void GIVEN_token_exchange_service_WHEN_started_with_empty_role_alias_THEN_server
202200
// set mock for port topic
203201
Topic portTopic = mock(Topic.class);
204202
when(portTopic.dflt(anyInt())).thenReturn(portTopic);
205-
when(portTopic.subscribe(any())).thenAnswer((a) -> {
206-
((Subscriber) a.getArgument(0)).published(WhatHappened.initialized, portTopic);
207-
return null;
208-
});
209-
when(portTopic.getOnce()).thenReturn(8080);
210-
211-
203+
212204
when(config.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC)).thenReturn(portTopic);
213205
when(configuration.lookup(SERVICES_NAMESPACE_TOPIC, DEFAULT_NUCLEUS_COMPONENT_NAME, CONFIGURATION_CONFIG_KEY,
214206
IOT_ROLE_ALIAS_TOPIC)).thenReturn(roleTopic);
@@ -240,11 +232,6 @@ void GIVEN_token_exchange_service_WHEN_auth_errors_THEN_server_errors_out(Extens
240232
// set mock for port topic
241233
Topic portTopic = mock(Topic.class);
242234
when(portTopic.dflt(anyInt())).thenReturn(portTopic);
243-
when(portTopic.subscribe(any())).thenAnswer((a) -> {
244-
((Subscriber) a.getArgument(0)).published(WhatHappened.initialized, portTopic);
245-
return null;
246-
});
247-
when(portTopic.getOnce()).thenReturn(8080);
248235
when(config.lookup(CONFIGURATION_CONFIG_KEY, PORT_TOPIC)).thenReturn(portTopic);
249236
when(configuration.lookup(SERVICES_NAMESPACE_TOPIC, DEFAULT_NUCLEUS_COMPONENT_NAME, CONFIGURATION_CONFIG_KEY,
250237
IOT_ROLE_ALIAS_TOPIC)).thenReturn(roleTopic);

0 commit comments

Comments
 (0)