Skip to content

refactor: change the tes topic names to be more user-friendly #1758

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

Merged
junfuchen99 merged 1 commit into from
Dec 23, 2025
Merged

refactor: change the tes topic names to be more user-friendly #1758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 16 additions & 16 deletions src/main/java/com/aws/greengrass/tes/TokenExchangeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ public class TokenExchangeService extends GreengrassService implements AwsCreden
private String iotRoleAlias;
private HttpServerImpl server;

public static final String CLOUD_4XX_ERROR_CACHE_TOPIC = "error4xxCredentialRetryInSec";
public static final String CLOUD_5XX_ERROR_CACHE_TOPIC = "error5xxCredentialRetryInSec";
public static final String UNKNOWN_ERROR_CACHE_TOPIC = "errorUnknownCredentialRetryInSec";
public static final String CREDENTIAL_RETRY_CONFIG_TOPIC = "credentialRetryInSec";
Copy link
Member

@aws-kevinrickard aws-kevinrickard Dec 23, 2025

Choose a reason for hiding this comment

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

Looking at this now, it should probably be credentialRetryDelaySeconds , to reduce ambiguity from other retry concepts which can also be measured in time such as jitter or timeouts.

And optionally dropping the In as we usually just list the unit (see other configs such as sessionExpirySeconds, although there are some other configs which do use In like InBytes, we don't appear to have a consistent standard one way or the other)

Copy link
Member

@aws-kevinrickard aws-kevinrickard Dec 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Or maybe credentialRequestErrorCacheSeconds would be more appropriate

public static final String CLOUD_4XX_ERROR_CACHE_TOPIC = "clientError";
public static final String CLOUD_5XX_ERROR_CACHE_TOPIC = "serverError";
public static final String UNKNOWN_ERROR_CACHE_TOPIC = "unknownError";
private static final int MINIMUM_ERROR_CACHE_IN_SEC = 10;
private static final int MAXIMUM_ERROR_CACHE_IN_SEC = 42_900;
private int cloud4xxErrorCache;
Expand Down Expand Up @@ -76,15 +77,15 @@ public TokenExchangeService(Topics topics,

cloud4xxErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
CLOUD_4XX_ERROR_CACHE_TOPIC)), CLOUD_4XX_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, CLOUD_4XX_ERROR_CACHE_TOPIC)), CLOUD_4XX_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);
cloud5xxErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
CLOUD_5XX_ERROR_CACHE_TOPIC)), CLOUD_5XX_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, CLOUD_5XX_ERROR_CACHE_TOPIC)), CLOUD_5XX_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);
unknownErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
UNKNOWN_ERROR_CACHE_TOPIC)), UNKNOWN_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, UNKNOWN_ERROR_CACHE_TOPIC)), UNKNOWN_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);

credentialRequestHandler.configureCacheSettings(cloud4xxErrorCache, cloud5xxErrorCache, unknownErrorCache);
Expand All @@ -94,21 +95,19 @@ public TokenExchangeService(Topics topics,
if (why.equals(WhatHappened.timestampUpdated)) {
return;
}
if (node != null && (node.childOf(CLOUD_4XX_ERROR_CACHE_TOPIC)
|| node.childOf(CLOUD_5XX_ERROR_CACHE_TOPIC)
|| node.childOf(UNKNOWN_ERROR_CACHE_TOPIC))) {
if (node != null && node.childOf(CREDENTIAL_RETRY_CONFIG_TOPIC)) {

int newCloud4xxErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
CLOUD_4XX_ERROR_CACHE_TOPIC)), CLOUD_4XX_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, CLOUD_4XX_ERROR_CACHE_TOPIC)), CLOUD_4XX_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);
int newCloud5xxErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
CLOUD_5XX_ERROR_CACHE_TOPIC)), CLOUD_5XX_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, CLOUD_5XX_ERROR_CACHE_TOPIC)), CLOUD_5XX_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);
int newUnknownErrorCache = validateErrorCacheConfig(Coerce.toInt(config.findOrDefault(
CredentialRequestHandler.DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY,
UNKNOWN_ERROR_CACHE_TOPIC)), UNKNOWN_ERROR_CACHE_TOPIC,
CREDENTIAL_RETRY_CONFIG_TOPIC, UNKNOWN_ERROR_CACHE_TOPIC)), UNKNOWN_ERROR_CACHE_TOPIC,
CredentialRequestHandler.DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);

if (cloud4xxErrorCache != newCloud4xxErrorCache
Expand All @@ -122,9 +121,9 @@ public TokenExchangeService(Topics topics,
credentialRequestHandler.configureCacheSettings(
newCloud4xxErrorCache, newCloud5xxErrorCache, newUnknownErrorCache);

logger.atInfo("tes-error-cache-config-change")
logger.atInfo("tes-credential-retry-config-change")
.kv("node", node).kv("why", why)
.log("TES error cache configuration updated");
.log("TES credential retry configuration updated");
}
}
if (node != null && node.childOf(PORT_TOPIC)) {
Expand Down Expand Up @@ -196,8 +195,9 @@ private void validateConfig() {
private int validateErrorCacheConfig(int newCacheValue, String topic, int defaultCacheValue) {
if (newCacheValue < MINIMUM_ERROR_CACHE_IN_SEC || newCacheValue > MAXIMUM_ERROR_CACHE_IN_SEC) {
logger.atError()
.log("Error cache value must be between {} and {} seconds, setting {} to default value {}",
MINIMUM_ERROR_CACHE_IN_SEC, MAXIMUM_ERROR_CACHE_IN_SEC, topic, defaultCacheValue);
.log("Credential retry value must be between {} and {} seconds, setting {}.{} to default value {}",
MINIMUM_ERROR_CACHE_IN_SEC, MAXIMUM_ERROR_CACHE_IN_SEC, CREDENTIAL_RETRY_CONFIG_TOPIC,
topic, defaultCacheValue);
return defaultCacheValue;
}
return newCacheValue;
Expand Down
37 changes: 19 additions & 18 deletions src/test/java/com/aws/greengrass/tes/TokenExchangeServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static com.aws.greengrass.tes.TokenExchangeService.ACTIVE_PORT_TOPIC;
import static com.aws.greengrass.tes.TokenExchangeService.CLOUD_4XX_ERROR_CACHE_TOPIC;
import static com.aws.greengrass.tes.TokenExchangeService.CLOUD_5XX_ERROR_CACHE_TOPIC;
import static com.aws.greengrass.tes.TokenExchangeService.CREDENTIAL_RETRY_CONFIG_TOPIC;
import static com.aws.greengrass.tes.TokenExchangeService.PORT_TOPIC;
import static com.aws.greengrass.tes.TokenExchangeService.TES_URI_ENV_VARIABLE_NAME;
import static com.aws.greengrass.tes.TokenExchangeService.TOKEN_EXCHANGE_SERVICE_TOPICS;
Expand Down Expand Up @@ -162,14 +163,14 @@ void GIVEN_token_exchange_service_WHEN_started_THEN_correct_env_set(int port) th
return null;
});

when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_4XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_4XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_5XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_5XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, UNKNOWN_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
UNKNOWN_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);

TokenExchangeService tes = new TokenExchangeService(config,
mockCredentialHandler,
Expand Down Expand Up @@ -219,14 +220,14 @@ void GIVEN_token_exchange_service_WHEN_started_with_empty_role_alias_THEN_server
when(configuration.lookup(SERVICES_NAMESPACE_TOPIC, DEFAULT_NUCLEUS_COMPONENT_NAME, CONFIGURATION_CONFIG_KEY,
IOT_ROLE_ALIAS_TOPIC)).thenReturn(roleTopic);

when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_4XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_4XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_5XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_5XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, UNKNOWN_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
UNKNOWN_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);

TokenExchangeService tes = spy(new TokenExchangeService(config,
mockCredentialHandler,
Expand Down Expand Up @@ -258,14 +259,14 @@ void GIVEN_token_exchange_service_WHEN_auth_errors_THEN_server_errors_out(Extens
when(configuration.lookup(SERVICES_NAMESPACE_TOPIC, DEFAULT_NUCLEUS_COMPONENT_NAME, CONFIGURATION_CONFIG_KEY,
IOT_ROLE_ALIAS_TOPIC)).thenReturn(roleTopic);

when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_4XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_4XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_4XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CLOUD_5XX_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
CLOUD_5XX_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_CLOUD_5XX_ERROR_CACHE_IN_SEC);

when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, UNKNOWN_ERROR_CACHE_TOPIC))
.thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);
when(config.findOrDefault(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC, CONFIGURATION_CONFIG_KEY, CREDENTIAL_RETRY_CONFIG_TOPIC,
UNKNOWN_ERROR_CACHE_TOPIC)).thenReturn(DEFAULT_UNKNOWN_ERROR_CACHE_IN_SEC);

TokenExchangeService tes = spy(new TokenExchangeService(config,
mockCredentialHandler,
Expand Down
Loading