Skip to content

Commit

Permalink
feat: added tests and some updates on orchestration service
Browse files Browse the repository at this point in the history
Signed-off-by: SimoneFiorani <[email protected]>
  • Loading branch information
sfiorani committed Mar 20, 2024
1 parent 701312b commit f0051a1
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,30 +146,33 @@ public void updated(Map<String, Object> properties) {
if (currentConfig.isEnforcementEnabled()) {
try {
startEnforcementMonitor();
enforceAlreadyRunningContainer();
} catch (Exception ex) {
logger.error("Error starting enforcement monitor, disconnecting from docker...", ex);
cleanUpDocker();
closeEnforcementMonitor();
logger.error("Disconnected from docker");
}

enforceAlreadyRunningContainer();
}
}

logger.info("Bundle {} has updated with config!", APP_ID);
}

private void startEnforcementMonitor() {
logger.info("Enforcement monitor starting...");
this.allowlistEnforcementMonitor = this.dockerClient.eventsCmd().withEventFilter("start")
.exec(new AllowlistEnforcementMonitor(currentConfig.getEnforcementAllowlist(), this));
logger.info("Enforcement monitor starting...done.");
}

private void closeEnforcementMonitor() {
try {
logger.info("Enforcement monitor closing...");
this.allowlistEnforcementMonitor.close();
this.allowlistEnforcementMonitor.awaitCompletion(5, TimeUnit.SECONDS);
this.allowlistEnforcementMonitor = null;
logger.info("Enforcement monitor closing...done.");
} catch (InterruptedException ex) {
logger.error("Waited too long to close enforcement monitor, stopping it...", ex);
Thread.currentThread().interrupt();
Expand All @@ -179,7 +182,9 @@ private void closeEnforcementMonitor() {
}

private void enforceAlreadyRunningContainer() {
logger.info("Enforcement check on already running containers...");
this.allowlistEnforcementMonitor.enforceAllowlistFor(listContainerDescriptors());
logger.info("Enforcement check on already running containers...done");
}

@Override
Expand Down Expand Up @@ -357,9 +362,7 @@ public String startContainer(ContainerConfiguration container) throws KuraExcept
logger.info("Creating new container instance");
pullImage(container.getImageConfiguration());
containerId = createContainer(container);
if (this.currentConfig.isEnforcementEnabled()) {
addContainerInstanceDigest(containerId, container.getEnforcementDigest());
}
addContainerInstanceDigest(containerId, container.getEnforcementDigest());
startContainer(containerId);
}

Expand All @@ -378,7 +381,11 @@ public void stopContainer(String id) throws KuraException {
checkRequestEnv(id);
try {
removeContainerInstanceDigest(id);
this.dockerClient.stopContainerCmd(id).exec();

if (listContainersIds().contains(id)) {
this.dockerClient.stopContainerCmd(id).exec();
}

} catch (Exception e) {
logger.error("Could not stop container {}. Caused by {}", id, e);
throw new KuraException(KuraErrorCode.OS_COMMAND_ERROR);
Expand All @@ -389,7 +396,11 @@ public void stopContainer(String id) throws KuraException {
public void deleteContainer(String id) throws KuraException {
checkRequestEnv(id);
try {
this.dockerClient.removeContainerCmd(id).exec();

if (listContainersIds().contains(id)) {
this.dockerClient.removeContainerCmd(id).exec();
}

this.frameworkManagedContainers.removeIf(c -> id.equals(c.id));
} catch (Exception e) {
logger.error("Could not remove container {}. Caused by {}", id, e);
Expand Down Expand Up @@ -528,8 +539,7 @@ private String createContainer(ContainerConfiguration containerDescription) thro
if (containerDescription.isContainerPrivileged()) {
configuration = configuration.withPrivileged(containerDescription.isContainerPrivileged());
}

commandBuilder.withExposedPorts(this.exposedPorts);
commandBuilder = commandBuilder.withExposedPorts(this.exposedPorts);

return commandBuilder.withHostConfig(configuration).exec().getId();

Expand Down Expand Up @@ -989,12 +999,15 @@ public Set<String> getImageDigestsByContainerName(String containerName) {
private void addContainerInstanceDigest(String containerId, Optional<String> containerInstanceDigest) {

if (containerInstanceDigest.isPresent()) {
logger.info("Container {} configuration presented enforcement digest. Adding it to the digests allowlist.",
logger.info(
"Container {} presented enforcement digest. Adding it to the digests allowlist: it will be used if the enforcement is enabled.",
containerId);
this.containerInstancesDigests.put(containerId, containerInstanceDigest.get());
} else {
logger.info("Container {} configuration doesn't contain the enforcement digest, but monitoring is enabled."
+ " Be sure that the digest is included in the service allowlist", containerId);
logger.info(
"Container {} doesn't contain the enforcement digest. "
+ "If enforcement is enabled, be sure that the digest is included in the service allowlist",
containerId);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@
</AD>

<AD
id="enforcement.allowlist"
name="Container Image Allowlist "
description="Image digest allowed to run, if the Allowlist Enforcement of the Container Orchestration Service is enabled. If not given, it will be computed by the Signature Verification of the service."
id="enforcement.digest"
name="Container Image Enforcement Digest "
description="Digest of the image wanted to be run, added to the Enforcement Allowlist of the Container Orchestration Service, if the enforcement monitoring is enabled. If not given, it will be computed by the Signature Verification of the service."
type="String" cardinality="1" required="false"
default="" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class ContainerInstanceOptions {
"");
private static final Property<Boolean> SIGNATURE_VERIFY_TLOG = new Property<>(
"container.signature.verify.transparency.log", true);
private static final Property<String> ENFORCEMENT_ALLOWLIST = new Property<>("enforcement.allowlist", "");
private static final Property<String> ENFORCEMENT_DIGEST = new Property<>("enforcement.digest", "");

private boolean enabled;
private final String image;
Expand Down Expand Up @@ -105,7 +105,7 @@ public class ContainerInstanceOptions {
private final Optional<String> signatureTrustAnchor;
private final Boolean signatureVerifyTransparencyLog;

private final Optional<String> enforcementAllowlist;
private final Optional<String> enforcementDigest;

public ContainerInstanceOptions(final Map<String, Object> properties) {
if (isNull(properties)) {
Expand Down Expand Up @@ -141,7 +141,7 @@ public ContainerInstanceOptions(final Map<String, Object> properties) {
this.containerRuntime = parseOptionalString(CONTAINER_RUNTIME.getOptional(properties));
this.signatureTrustAnchor = parseOptionalString(SIGNATURE_TRUST_ANCHOR.getOptional(properties));
this.signatureVerifyTransparencyLog = SIGNATURE_VERIFY_TLOG.get(properties);
this.enforcementAllowlist = parseOptionalString(ENFORCEMENT_ALLOWLIST.getOptional(properties));
this.enforcementDigest = parseOptionalString(ENFORCEMENT_DIGEST.getOptional(properties));
}

private Map<String, String> parseVolume(String volumeString) {
Expand Down Expand Up @@ -357,8 +357,8 @@ public Boolean getSignatureVerifyTransparencyLog() {
return this.signatureVerifyTransparencyLog;
}

public Optional<String> getEnforcementAllowlist() {
return this.enforcementAllowlist;
public Optional<String> getEnforcementDigest() {
return this.enforcementDigest;
}

private ImageConfiguration buildImageConfig() {
Expand Down Expand Up @@ -391,7 +391,7 @@ public ContainerConfiguration getContainerConfiguration() {
.setContainerNetowrkConfiguration(buildContainerNetworkConfig())
.setLoggerParameters(getLoggerParameters()).setEntryPoint(getEntryPoint())
.setRestartOnFailure(getRestartOnFailure()).setMemory(getMemory()).setCpus(getCpus()).setGpus(getGpus())
.setRuntime(getRuntime()).setEnforcementDigest(getEnforcementAllowlist()).build();
.setRuntime(getRuntime()).setEnforcementDigest(getEnforcementDigest()).build();
}

private List<Integer> parsePortString(String ports) {
Expand Down Expand Up @@ -438,7 +438,7 @@ public int hashCode() {
return Objects.hash(containerCpus, containerDevice, containerEntryPoint, containerEnv, containerGpus,
containerLoggerType, containerLoggingParameters, containerMemory, containerName,
containerNetworkingMode, containerPortProtocol, containerRuntime, containerVolumeString,
containerVolumes, enabled, enforcementAllowlist, externalPorts, image, imageDownloadTimeout, imageTag,
containerVolumes, enabled, enforcementDigest, externalPorts, image, imageDownloadTimeout, imageTag,
internalPorts, maxDownloadRetries, privilegedMode, registryPassword, registryURL, registryUsername,
restartOnFailure, retryInterval, signatureTrustAnchor, signatureVerifyTransparencyLog);
}
Expand Down Expand Up @@ -469,7 +469,7 @@ public boolean equals(Object obj) {
&& Objects.equals(containerRuntime, other.containerRuntime)
&& Objects.equals(containerVolumeString, other.containerVolumeString)
&& Objects.equals(containerVolumes, other.containerVolumes) && enabled == other.enabled
&& Objects.equals(enforcementAllowlist, other.enforcementAllowlist)
&& Objects.equals(enforcementDigest, other.enforcementDigest)
&& Objects.equals(externalPorts, other.externalPorts) && Objects.equals(image, other.image)
&& imageDownloadTimeout == other.imageDownloadTimeout && Objects.equals(imageTag, other.imageTag)
&& Objects.equals(internalPorts, other.internalPorts) && maxDownloadRetries == other.maxDownloadRetries
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,27 @@
package org.eclipse.kura.container.orchestration.provider;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import org.eclipse.kura.KuraException;
import org.eclipse.kura.configuration.Password;
Expand All @@ -42,6 +49,7 @@

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.CreateContainerCmd;
import com.github.dockerjava.api.command.CreateContainerResponse;
import com.github.dockerjava.api.command.ListContainersCmd;
import com.github.dockerjava.api.command.ListImagesCmd;
import com.github.dockerjava.api.model.Container;
Expand All @@ -50,6 +58,7 @@

public class ContainerOrchestrationServiceImplTest {

private static final String CONTAINER_INSTANCE_DIGEST = "sha256:test";
private static final String[] REPO_DIGESTS_ARRAY = new String[] {
"ubuntu@sha256:c26ae7472d624ba1fafd296e73cecc4f93f853088e6a9c13c0d52f6ca5865107" };
private static final String[] EXPECTED_DIGESTS_ARRAY = new String[] {
Expand All @@ -70,6 +79,8 @@ public class ContainerOrchestrationServiceImplTest {
private static final String REPOSITORY_USERNAME = "repository.username";
private static final String REPOSITORY_PASSWORD = "repository.password";

private static final String ENFORCEMENT_ENABLED = "enforcement.enabled";

private static final String REGISTRY_URL = "https://test";
private static final String REGISTRY_USERNAME = "test";
private static final String REGISTRY_PASSWORD = "test1";
Expand Down Expand Up @@ -99,7 +110,10 @@ public class ContainerOrchestrationServiceImplTest {
private Map<String, Object> properties;
private String containerId;

List<String> digestsList;
CreateContainerCmd createContainerCmd = mock(CreateContainerCmd.class);
CreateContainerResponse createContainerResponse = mock(CreateContainerResponse.class);

Set<String> digestsList;

@Test
public void testServiceActivateEmptyProperties() {
Expand Down Expand Up @@ -317,6 +331,41 @@ public void testImageDigestsByContainerName() throws KuraException, InterruptedE

}

@Test
public void testContainerInstanceDigestIsAddedToAllowlist() throws KuraException, InterruptedException {

givenFullProperties(true);
givenEnforcementEnabledProperty(true);
givenDockerServiceImplSpyForContainerInstancesDigests();
givenDockerClient();

whenActivateInstance();
whenDockerClientMockCreateContainer();
whenMockForContainerInstancesDigestAdding();
whenRunContainer();

thenContainerInstanceDigestIsAddedToAllowlist();
thenContainerInstanceDigestIsExpectedOne(CONTAINER_INSTANCE_DIGEST);
}

@Test
public void testContainerInstanceDigestIsAddedToAndRemovedFromAllowlist()
throws KuraException, InterruptedException {

givenFullProperties(true);
givenEnforcementEnabledProperty(true);
givenDockerServiceImplSpyForContainerInstancesDigests();
givenDockerClient();

whenActivateInstance();
whenDockerClientMockCreateContainer();
whenMockForContainerInstancesDigestAdding();
whenRunContainer();
whenStopContainer();

thenContainerInstanceDigestIsNotInAllowlist();
}

/**
* givens
*/
Expand All @@ -330,6 +379,12 @@ private void givenDockerServiceImplSpy() throws KuraException, InterruptedExcept
Mockito.doNothing().when(this.dockerService).pullImage(any(ImageConfiguration.class));
}

private void givenDockerServiceImplSpyForContainerInstancesDigests() throws KuraException, InterruptedException {
this.dockerService = Mockito.spy(new ContainerOrchestrationServiceImpl());
doNothing().when(this.dockerService).pullImage(any(ImageConfiguration.class));

}

private void givenDockerClient() {
this.localDockerClient = mock(DockerClient.class, Mockito.RETURNS_DEEP_STUBS);
this.dockerService.setDockerClient(this.localDockerClient);
Expand Down Expand Up @@ -378,6 +433,10 @@ private void givenAuthWithRepoAndCredentials() {
this.properties.put(IMAGES_DOWNLOAD_TIMEOUT, DEFAULT_IMAGES_DOWNLOAD_TIMEOUT);
}

private void givenEnforcementEnabledProperty(boolean enabled) {
this.properties.put(ENFORCEMENT_ENABLED, enabled);
}

/**
* when
*/
Expand Down Expand Up @@ -442,6 +501,19 @@ private void whenDockerClientMockSomeContainers() {
when(this.mockedListContainersCmd.exec()).thenReturn(containerListmock);
}

private void whenDockerClientMockCreateContainer() {

this.createContainerCmd = mock(CreateContainerCmd.class, Mockito.RETURNS_DEEP_STUBS);
this.createContainerResponse = new CreateContainerResponse();
this.createContainerResponse.setId("CIAO");
when(this.localDockerClient.createContainerCmd(anyString())).thenReturn(this.createContainerCmd);
when(this.createContainerCmd.withHostConfig(any())).thenReturn(this.createContainerCmd);
when(this.createContainerCmd.withHostConfig(any()).exec()).thenReturn(this.createContainerResponse);
when(this.createContainerCmd.withExposedPorts(anyList())).thenReturn(this.createContainerCmd);
when(this.createContainerCmd.withName(any())).thenReturn(this.createContainerCmd);

}

private void whenMockforContainerCreation() {

// Build Respective CD's
Expand Down Expand Up @@ -520,13 +592,33 @@ REGISTRY_USERNAME, new Password(REGISTRY_PASSWORD))))

}

private void whenMockForContainerInstancesDigestAdding() {

// Build Respective CD's
ContainerInstanceDescriptor mcontCD1 = ContainerInstanceDescriptor.builder().setContainerID("1d3dewf34r5")
.setContainerName(CONTAINER_NAME_FRANK).setContainerImage(IMAGE_NAME_NGINX).build();

this.imageConfig = new ImageConfiguration.ImageConfigurationBuilder().setImageName(IMAGE_NAME_NGINX)
.setImageTag(IMAGE_TAG_LATEST).setImageDownloadTimeoutSeconds(0)
.setRegistryCredentials(Optional.of(new PasswordRegistryCredentials(Optional.of(REGISTRY_URL),
REGISTRY_USERNAME, new Password(REGISTRY_PASSWORD))))
.build();

this.containerConfig1 = ContainerConfiguration.builder().setContainerName(CONTAINER_NAME_FRANK)
.setImageConfiguration(imageConfig).setVolumes(Collections.singletonMap("test", "~/test/test"))
.setEnforcementDigest(Optional.of(CONTAINER_INSTANCE_DIGEST)).setLoggingType("NONE").build();

this.runningContainerDescriptor = new ContainerInstanceDescriptor[] { mcontCD1 };

}

private void whenRunContainer() throws KuraException, InterruptedException {
// startContainer
this.containerId = this.dockerService.startContainer(this.containerConfig1);
}

private void whenStopContainer() throws KuraException {
// startContainer
// stopContainer
this.dockerService.stopContainer(this.containerId);
}

Expand Down Expand Up @@ -592,6 +684,22 @@ private void thenCheckIfImagesWereListed() {
}

private void thenDigestsListEqualsExpectedOne(String[] digestsArray) {
assertEquals(this.digestsList, Arrays.asList(digestsArray));
assertEquals(this.digestsList, new HashSet<>(Arrays.asList(digestsArray)));
}

private void thenContainerInstanceDigestIsAddedToAllowlist() {
assertFalse(this.dockerService.getContainerInstancesAllowlist().isEmpty());
}

private void thenContainerInstanceDigestIsNotInAllowlist() {
this.dockerService.getContainerInstancesAllowlist().stream().forEach(System.err::println);
assertTrue(this.dockerService.getContainerInstancesAllowlist().isEmpty());
}

private void thenContainerInstanceDigestIsExpectedOne(String expected) {

List<String> actualDigests = new ArrayList<>(this.dockerService.getContainerInstancesAllowlist());
assertEquals(actualDigests.get(0), expected);

}
}
Loading

0 comments on commit f0051a1

Please sign in to comment.