Skip to content

Commit a542a38

Browse files
MMaieroMMaiero
committed
fix(container.provider): generate valid temporary identity names with retries
1 parent 3c59d91 commit a542a38

2 files changed

Lines changed: 159 additions & 17 deletions

File tree

‎kura/org.eclipse.kura.container.provider/src/main/java/org/eclipse/kura/container/provider/ContainerInstance.java‎

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.stream.Collectors;
4040

4141
import org.eclipse.kura.KuraException;
42+
import org.eclipse.kura.KuraErrorCode;
4243
import org.eclipse.kura.configuration.ComponentConfiguration;
4344
import org.eclipse.kura.configuration.ConfigurableComponent;
4445
import org.eclipse.kura.configuration.ConfigurationService;
@@ -67,6 +68,9 @@ public class ContainerInstance implements ConfigurableComponent, ContainerOrches
6768
private static final Logger logger = LoggerFactory.getLogger(ContainerInstance.class);
6869

6970
private static final ValidationResult FAILED_VALIDATION = new ValidationResult();
71+
private static final String CONTAINER_IDENTITY_PREFIX = "container_";
72+
private static final int MAX_IDENTITY_NAME_LENGTH = 255;
73+
private static final int MAX_IDENTITY_NAME_GENERATION_ATTEMPTS = 10;
7074

7175
private final ExecutorService executor = Executors.newSingleThreadExecutor();
7276

@@ -412,23 +416,11 @@ private void createTemporaryIdentityIfEnabled(final ContainerInstanceOptions opt
412416
final Set<Permission> permissions = options.getContainerPermissions().stream().map(Permission::new)
413417
.collect(Collectors.toSet());
414418

415-
final String identityName = "container_" + options.getContainerName().replace("-", "_");
416-
417419
// Generate password
418420
final String password = new String(PasswordGenerator
419421
.generatePassword(passwordStrengthVerificationService.getPasswordStrengthRequirements()));
420422

421-
// Create identity configuration (computePasswordHash will clear this char[])
422-
final PasswordConfiguration passwordConfiguration = new PasswordConfiguration(false, true,
423-
Optional.of(password.toCharArray()), Optional.empty());
424-
final AssignedPermissions assignedPermissions = new AssignedPermissions(permissions);
425-
final IdentityConfiguration configuration = new IdentityConfiguration(identityName,
426-
Arrays.asList(passwordConfiguration, assignedPermissions));
427-
428-
// Create temporary identity with very long lifetime (365 days)
429-
// The identity lifetime matches the container lifecycle - cleanup happens when container stops
430-
// The duration is a safety net for cases where cleanup fails
431-
ContainerInstance.this.identityService.createTemporaryIdentity(configuration, Duration.ofDays(365));
423+
final String identityName = createTemporaryIdentityWithValidName(options, permissions, password);
432424

433425
// Store identity name and password for env injection (fresh char[] from same string)
434426
ContainerInstance.this.currentTemporaryIdentityName.set(identityName);
@@ -445,6 +437,76 @@ private void createTemporaryIdentityIfEnabled(final ContainerInstanceOptions opt
445437
}
446438
}
447439

440+
private String createTemporaryIdentityWithValidName(final ContainerInstanceOptions options,
441+
final Set<Permission> permissions, final String password) throws KuraException {
442+
443+
final String baseIdentityName = sanitizeContainerIdentityName(options.getContainerName());
444+
445+
for (int attempt = 0; attempt < MAX_IDENTITY_NAME_GENERATION_ATTEMPTS; attempt++) {
446+
final String candidateName = buildIdentityNameCandidate(baseIdentityName, attempt);
447+
448+
try {
449+
final PasswordConfiguration passwordConfiguration = new PasswordConfiguration(false, true,
450+
Optional.of(password.toCharArray()), Optional.empty());
451+
final AssignedPermissions assignedPermissions = new AssignedPermissions(permissions);
452+
final IdentityConfiguration configuration = new IdentityConfiguration(candidateName,
453+
Arrays.asList(passwordConfiguration, assignedPermissions));
454+
455+
ContainerInstance.this.identityService.createTemporaryIdentity(configuration, Duration.ofDays(365));
456+
return candidateName;
457+
458+
} catch (final KuraException e) {
459+
if (!shouldRetryIdentityNameGeneration(e, attempt)) {
460+
throw e;
461+
}
462+
}
463+
}
464+
465+
throw new KuraException(KuraErrorCode.INTERNAL_ERROR,
466+
"Unable to generate a valid temporary identity name for container " + options.getContainerName());
467+
}
468+
469+
private String sanitizeContainerIdentityName(final String containerName) {
470+
String safeName = containerName.replaceAll("[^a-zA-Z0-9._]", "_");
471+
safeName = safeName.replaceAll("[._]{2,}", "_");
472+
safeName = safeName.replaceAll("^[._]+", "").replaceAll("[._]+$", "");
473+
474+
if (safeName.isEmpty()) {
475+
safeName = "auto";
476+
}
477+
478+
return CONTAINER_IDENTITY_PREFIX + safeName;
479+
}
480+
481+
private String buildIdentityNameCandidate(final String baseIdentityName, final int attempt) {
482+
final String candidate = attempt == 0 ? baseIdentityName : baseIdentityName + "_" + attempt;
483+
484+
if (candidate.length() <= MAX_IDENTITY_NAME_LENGTH) {
485+
return candidate;
486+
}
487+
488+
if (attempt == 0) {
489+
return candidate.substring(0, MAX_IDENTITY_NAME_LENGTH);
490+
}
491+
492+
final String suffix = "_" + attempt;
493+
return candidate.substring(0, MAX_IDENTITY_NAME_LENGTH - suffix.length()) + suffix;
494+
}
495+
496+
private boolean shouldRetryIdentityNameGeneration(final KuraException e, final int attempt) {
497+
if (attempt == MAX_IDENTITY_NAME_GENERATION_ATTEMPTS - 1) {
498+
return false;
499+
}
500+
501+
if (!KuraErrorCode.INVALID_PARAMETER.equals(e.getCode())) {
502+
return false;
503+
}
504+
505+
final String message = e.getMessage();
506+
return message != null && (message.contains("Identity name") || message.contains("identity with name")
507+
|| message.contains("already exists"));
508+
}
509+
448510
@Override
449511
public State onConfigurationUpdated(ContainerInstanceOptions newOptions) {
450512
if (newOptions.equals(this.options)) {

‎kura/test/org.eclipse.kura.container.provider.test/src/test/java/org/eclipse/kura/container/provider/ContainerIdentityIntegrationTest.java‎

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Set;
3434
import java.util.concurrent.CountDownLatch;
3535
import java.util.concurrent.TimeUnit;
36+
import java.util.concurrent.atomic.AtomicBoolean;
3637
import java.util.concurrent.atomic.AtomicInteger;
3738
import java.util.concurrent.atomic.AtomicReference;
3839

@@ -66,6 +67,7 @@ public class ContainerIdentityIntegrationTest {
6667
private static final String CONTAINER_ID = "container-id";
6768
private static final String SECOND_CONTAINER_ID = "container-id-2";
6869
private static final String UPDATED_CONTAINER_NAME = CONTAINER_NAME + "-v2";
70+
private static final String INVALID_CONTAINER_NAME = "....@@@....";
6971

7072
private ContainerInstance containerInstance;
7173
private ContainerOrchestrationService containerOrchestrationService;
@@ -213,14 +215,42 @@ public void clearsTemporaryPasswordAfterCleanupTemporaryIdentity() throws Except
213215
thenTemporaryPasswordIsCleared();
214216
}
215217

218+
@Test
219+
public void generatesValidTemporaryIdentityNameFromInvalidContainerName() throws Exception {
220+
givenIdentityIntegrationIsEnabled(INVALID_CONTAINER_NAME);
221+
givenTemporaryIdentityServiceProvidesPassword();
222+
givenContainerOrchestratorStartsSuccessfully();
223+
224+
whenContainerInstanceIsActivated();
225+
226+
thenTemporaryIdentityUsesName("container_auto");
227+
thenStartContainerReceivesTokenEnvironment();
228+
}
229+
230+
@Test
231+
public void retriesTemporaryIdentityCreationWhenNameAlreadyExists() throws Exception {
232+
givenIdentityIntegrationIsEnabled();
233+
givenTemporaryIdentityServiceRetriesNameConflict();
234+
givenContainerOrchestratorStartsSuccessfully();
235+
236+
whenContainerInstanceIsActivated();
237+
238+
thenTemporaryIdentityCreationIsRetriedWithSuffixedName();
239+
thenStartContainerReceivesTokenEnvironment();
240+
}
241+
216242
/*
217243
* GIVEN
218244
*/
219245

220246
private void givenIdentityIntegrationIsEnabled() {
247+
givenIdentityIntegrationIsEnabled(CONTAINER_NAME);
248+
}
249+
250+
private void givenIdentityIntegrationIsEnabled(final String containerName) {
221251
this.properties.put("container.enabled", true);
222-
this.properties.put("container.name", CONTAINER_NAME);
223-
this.properties.put("kura.service.pid", CONTAINER_NAME);
252+
this.properties.put("container.name", containerName);
253+
this.properties.put("kura.service.pid", containerName);
224254
this.properties.put("container.image", CONTAINER_IMAGE);
225255
this.properties.put("container.image.tag", CONTAINER_TAG);
226256
this.properties.put("container.identity.enabled", true);
@@ -269,6 +299,35 @@ private void givenContainerOrchestratorStartsSuccessfullyWithIds(final String...
269299
}).when(this.containerOrchestrationService).startContainer(this.configurationCaptor.capture());
270300
}
271301

302+
private void givenTemporaryIdentityServiceRetriesNameConflict() throws Exception {
303+
final AtomicBoolean conflictRaised = new AtomicBoolean(false);
304+
305+
doAnswer(invocation -> {
306+
this.createCount.incrementAndGet();
307+
308+
final IdentityConfiguration config = invocation.getArgument(0);
309+
this.capturedIdentityNames.add(config.getName());
310+
final char[] newPassword = config.getComponent(PasswordConfiguration.class).orElseThrow()
311+
.getNewPassword().orElseThrow();
312+
this.capturedPasswords.add(new String(newPassword));
313+
314+
final String expectedBaseName = "container_" + CONTAINER_NAME.replace("-", "_");
315+
if (!conflictRaised.get() && expectedBaseName.equals(config.getName())) {
316+
conflictRaised.set(true);
317+
throw new KuraException(KuraErrorCode.INVALID_PARAMETER,
318+
"An identity with name '" + config.getName() + "' already exists");
319+
}
320+
321+
return null;
322+
}).when(this.identityService).createTemporaryIdentity(
323+
this.identityConfigCaptor.capture(), this.durationCaptor.capture());
324+
325+
doAnswer(invocation -> {
326+
this.deleteCount.incrementAndGet();
327+
return true;
328+
}).when(this.identityService).deleteIdentity(anyString());
329+
}
330+
272331
private void givenTemporaryPasswordIsSet() throws Exception {
273332
final Field field = ContainerInstance.class.getDeclaredField("currentTemporaryPassword");
274333
field.setAccessible(true);
@@ -374,16 +433,28 @@ private void thenTemporaryIdentityIsCreatedWithPermissions() throws Exception {
374433
permissions.stream().anyMatch(permission -> "rest.write".equals(permission.getName())));
375434
}
376435

436+
private void thenTemporaryIdentityUsesName(final String expectedIdentityName) throws Exception {
437+
awaitCounterAtLeast(this.createCount, 1);
438+
439+
verify(this.identityService).createTemporaryIdentity(
440+
this.identityConfigCaptor.capture(), this.durationCaptor.capture());
441+
442+
final IdentityConfiguration config = this.identityConfigCaptor.getValue();
443+
assertEquals("Identity name should be normalized", expectedIdentityName, config.getName());
444+
}
445+
377446
private void thenStartContainerReceivesTokenEnvironment() throws Exception {
378447
assertTrue("Container start was not invoked", this.startLatch.await(2, TimeUnit.SECONDS));
379448

380449
final ContainerConfiguration configuration = this.configurationCaptor.getValue();
381450
final List<String> envVars = configuration.getContainerEnvVars();
451+
final String latestIdentityName = this.capturedIdentityNames.get(this.capturedIdentityNames.size() - 1);
452+
final String latestPassword = this.capturedPasswords.get(this.capturedPasswords.size() - 1);
382453

383454
assertTrue("Identity name var missing",
384-
envVars.contains("KURA_IDENTITY_NAME=" + this.capturedIdentityNames.get(0)));
455+
envVars.contains("KURA_IDENTITY_NAME=" + latestIdentityName));
385456
assertTrue("Password var missing",
386-
envVars.contains("KURA_IDENTITY_PASSWORD=" + this.capturedPasswords.get(0)));
457+
envVars.contains("KURA_IDENTITY_PASSWORD=" + latestPassword));
387458
// Check that KURA_REST_BASE_URL is set (now dynamic, not hardcoded to localhost:8080)
388459
assertTrue("Base URL env var missing",
389460
envVars.stream().anyMatch(envVar -> envVar.startsWith("KURA_REST_BASE_URL=")));
@@ -424,6 +495,15 @@ private void thenLatestContainerStartReceivesRefreshedPassword() {
424495
envVars.contains("KURA_IDENTITY_PASSWORD=" + latestPassword));
425496
}
426497

498+
private void thenTemporaryIdentityCreationIsRetriedWithSuffixedName() throws Exception {
499+
awaitCounterAtLeast(this.createCount, 2);
500+
501+
final String expectedBaseName = "container_" + CONTAINER_NAME.replace("-", "_");
502+
assertEquals("First identity creation should use base name", expectedBaseName, this.capturedIdentityNames.get(0));
503+
assertEquals("Second identity creation should use suffixed name", expectedBaseName + "_1",
504+
this.capturedIdentityNames.get(1));
505+
}
506+
427507
private void thenTemporaryPasswordIsCleared() throws Exception {
428508
final Field field = ContainerInstance.class.getDeclaredField("currentTemporaryPassword");
429509
field.setAccessible(true);

0 commit comments

Comments
 (0)