Skip to content

Commit

Permalink
machine.getOwner() should return userId and not a namespace
Browse files Browse the repository at this point in the history 
Fix #3042

Change-Id: I64b4cd3931fec502073ead0115a312430f2434b8
Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>
  • Loading branch information
@benoitf
benoitf committed Dec 13, 2016
1 parent 5be04c1 commit 88a9a7e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 48 deletions.
3 changes: 2 additions & 1 deletion ...cker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/MachineProviderImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ public Instance startService(String namespace,
service.getId(),
workspaceId);

final String userId = EnvironmentContext.getCurrent().getSubject().getUserId();
MachineImpl machine = new MachineImpl(MachineConfigImpl.builder()
.setDev(isDev)
.setName(machineName)
Expand All @@ -323,7 +324,7 @@ public Instance startService(String namespace,
service.getId(),
workspaceId,
envName,
namespace,
userId,
MachineStatus.RUNNING,
null);

Expand Down
22 changes: 3 additions & 19 deletions ...ine-ext-server/src/main/java/org/eclipse/che/ide/ext/machine/server/ssh/KeysInjector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@

import org.eclipse.che.api.core.NotFoundException;
import org.eclipse.che.api.core.ServerException;
import org.eclipse.che.api.core.model.user.User;
import org.eclipse.che.api.core.notification.EventService;
import org.eclipse.che.api.core.notification.EventSubscriber;
import org.eclipse.che.api.environment.server.CheEnvironmentEngine;
import org.eclipse.che.api.machine.server.spi.Instance;
import org.eclipse.che.api.machine.shared.dto.event.MachineStatusEvent;
import org.eclipse.che.api.ssh.server.SshManager;
import org.eclipse.che.api.ssh.server.model.impl.SshPairImpl;
import org.eclipse.che.api.user.server.UserManager;
import org.eclipse.che.plugin.docker.client.DockerConnector;
import org.eclipse.che.plugin.docker.client.Exec;
import org.eclipse.che.plugin.docker.client.LogMessage;
Expand Down Expand Up @@ -49,21 +47,18 @@ public class KeysInjector {
private final EventService eventService;
private final DockerConnector docker;
private final SshManager sshManager;
private final UserManager userManager;
// TODO replace with WorkspaceManager
private final CheEnvironmentEngine environmentEngine;

@Inject
public KeysInjector(EventService eventService,
DockerConnector docker,
SshManager sshManager,
CheEnvironmentEngine environmentEngine,
UserManager userManager) {
CheEnvironmentEngine environmentEngine) {
this.eventService = eventService;
this.docker = docker;
this.sshManager = sshManager;
this.environmentEngine = environmentEngine;
this.userManager = userManager;
}

@PostConstruct
Expand All @@ -82,19 +77,8 @@ public void onEvent(MachineStatusEvent event) {
}

try {
// get userid
final String userId;
try {
final User user = userManager.getByName(machine.getOwner());
userId = user.getId();
} catch (NotFoundException e) {
LOG.error("User with name {} associated to machine not found", machine.getOwner());
return;
}


// get machine keypairs
List<SshPairImpl> sshPairs = sshManager.getPairs(userId, "machine");
List<SshPairImpl> sshPairs = sshManager.getPairs(machine.getOwner(), "machine");
final List<String> publicMachineKeys = sshPairs.stream()
.filter(sshPair -> sshPair.getPublicKey() != null)
.map(SshPairImpl::getPublicKey)
Expand All @@ -103,7 +87,7 @@ public void onEvent(MachineStatusEvent event) {
// get workspace keypair (if any)
SshPairImpl sshWorkspacePair = null;
try {
sshWorkspacePair = sshManager.getPair(userId, "workspace", event.getWorkspaceId());
sshWorkspacePair = sshManager.getPair(machine.getOwner(), "workspace", event.getWorkspaceId());
} catch (NotFoundException e) {
LOG.debug("No ssh key associated to the workspace", e);
}
Expand Down
49 changes: 21 additions & 28 deletions ...ext-server/src/test/java/org/eclipse/che/ide/ext/machine/server/ssh/KeysInjectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.eclipse.che.api.machine.shared.dto.event.MachineStatusEvent;
import org.eclipse.che.api.ssh.server.SshManager;
import org.eclipse.che.api.ssh.server.model.impl.SshPairImpl;
import org.eclipse.che.api.user.server.UserManager;
import org.eclipse.che.plugin.docker.client.DockerConnector;
import org.eclipse.che.plugin.docker.client.Exec;
import org.eclipse.che.plugin.docker.client.LogMessage;
Expand Down Expand Up @@ -61,8 +60,7 @@
public class KeysInjectorTest {
private static final String WORKSPACE_ID = "workspace123";
private static final String MACHINE_ID = "machine123";
private static final String OWNER_NAME = "user";
private static final String USER_ID = "user123";
private static final String OWNER = "user123";
private static final String CONTAINER_ID = "container123";
private static final String EXEC_ID = "exec123";

Expand Down Expand Up @@ -91,8 +89,6 @@ public class KeysInjectorTest {
@Mock
SshManager sshManager;
@Mock
UserManager userManager;
@Mock
User user;

EventSubscriber<MachineStatusEvent> subscriber;
Expand All @@ -107,17 +103,14 @@ public void setUp() throws Exception {
when(machineRuntime.getProperties()).thenReturn(metadataProperties);

when(environmentEngine.getMachine(WORKSPACE_ID, MACHINE_ID)).thenReturn(instance);
when(instance.getOwner()).thenReturn(OWNER_NAME);
when(instance.getOwner()).thenReturn(OWNER);
when(instance.getRuntime()).thenReturn(machineRuntime);
when(instance.getLogger()).thenReturn(lineConsumer);

keysInjector.start();
verify(eventService).subscribe(subscriberCaptor.capture());
subscriber = subscriberCaptor.getValue();

when(userManager.getByName(eq(OWNER_NAME))).thenReturn(user);
when(user.getId()).thenReturn(USER_ID);

when(docker.createExec(any(CreateExecParams.class))).thenReturn(exec);
when(exec.getId()).thenReturn(EXEC_ID);
}
Expand All @@ -138,39 +131,39 @@ public void shouldNotInjectSshKeysWhenThereAreNotAnyPair() throws Exception {
.withWorkspaceId(WORKSPACE_ID));

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));
verifyZeroInteractions(docker, environmentEngine, sshManager);
}

@Test
public void shouldNotInjectSshKeysWhenThereAreNotAnyPairWithPublicKey() throws Exception {
when(sshManager.getPairs(anyString(), anyString()))
.thenReturn(Collections.singletonList(new SshPairImpl(USER_ID, "machine", "myPair", null, null)));
.thenReturn(Collections.singletonList(new SshPairImpl(OWNER, "machine", "myPair", null, null)));

subscriber.onEvent(newDto(MachineStatusEvent.class).withEventType(MachineStatusEvent.EventType.RUNNING)
.withMachineId(MACHINE_ID)
.withWorkspaceId(WORKSPACE_ID));

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));
verifyZeroInteractions(docker, environmentEngine, sshManager);
}

@Test
public void shouldInjectSshKeysWhenThereAreAnyPairWithNotNullPublicKey() throws Exception {
when(sshManager.getPairs(anyString(), anyString()))
.thenReturn(Arrays.asList(new SshPairImpl(USER_ID, "machine", "myPair", "publicKey1", null),
new SshPairImpl(USER_ID, "machine", "myPair", "publicKey2", null)));
.thenReturn(Arrays.asList(new SshPairImpl(OWNER, "machine", "myPair", "publicKey1", null),
new SshPairImpl(OWNER, "machine", "myPair", "publicKey2", null)));

subscriber.onEvent(newDto(MachineStatusEvent.class).withEventType(MachineStatusEvent.EventType.RUNNING)
.withMachineId(MACHINE_ID)
.withWorkspaceId(WORKSPACE_ID));

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));

ArgumentCaptor<CreateExecParams> argumentCaptor = ArgumentCaptor.forClass(CreateExecParams.class);
verify(docker).createExec(argumentCaptor.capture());
Expand All @@ -193,7 +186,7 @@ public void shouldInjectSshKeysWhenThereIsOnlyWorkspaceKey() throws Exception {

// workspace keypair
when(sshManager.getPair(anyString(), eq("workspace"), anyString()))
.thenReturn(new SshPairImpl(USER_ID, "workspace", WORKSPACE_ID, "publicKeyWorkspace", null));
.thenReturn(new SshPairImpl(OWNER, "workspace", WORKSPACE_ID, "publicKeyWorkspace", null));


subscriber.onEvent(newDto(MachineStatusEvent.class).withEventType(MachineStatusEvent.EventType.RUNNING)
Expand All @@ -202,8 +195,8 @@ public void shouldInjectSshKeysWhenThereIsOnlyWorkspaceKey() throws Exception {

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
// check calls for machine and workspace ssh pairs
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));

ArgumentCaptor<CreateExecParams> argumentCaptor = ArgumentCaptor.forClass(CreateExecParams.class);
verify(docker).createExec(argumentCaptor.capture());
Expand All @@ -221,11 +214,11 @@ public void shouldInjectSshKeysWhenThereIsOnlyWorkspaceKey() throws Exception {
public void shouldInjectSshKeysWhenThereIsNoPublicWorkspaceKeyButMachineKeys() throws Exception {
// no machine key pairs
when(sshManager.getPairs(anyString(), eq("machine")))
.thenReturn(Arrays.asList(new SshPairImpl(USER_ID, "machine", "myPair", "publicKey1", null)));
.thenReturn(Arrays.asList(new SshPairImpl(OWNER, "machine", "myPair", "publicKey1", null)));

// workspace keypair without public key
when(sshManager.getPair(anyString(), eq("workspace"), anyString()))
.thenReturn(new SshPairImpl(USER_ID, "workspace", WORKSPACE_ID, null, null));
.thenReturn(new SshPairImpl(OWNER, "workspace", WORKSPACE_ID, null, null));


subscriber.onEvent(newDto(MachineStatusEvent.class).withEventType(MachineStatusEvent.EventType.RUNNING)
Expand All @@ -234,8 +227,8 @@ public void shouldInjectSshKeysWhenThereIsNoPublicWorkspaceKeyButMachineKeys() t

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
// check calls for machine and workspace ssh pairs
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));

ArgumentCaptor<CreateExecParams> argumentCaptor = ArgumentCaptor.forClass(CreateExecParams.class);
verify(docker).createExec(argumentCaptor.capture());
Expand Down Expand Up @@ -266,16 +259,16 @@ public void shouldNotInjectSshKeysWhenThereIsNoWorkspaceKey() throws Exception {

verify(environmentEngine).getMachine(eq(WORKSPACE_ID), eq(MACHINE_ID));
// check calls for machine and workspace ssh pairs
verify(sshManager).getPairs(eq(USER_ID), eq("machine"));
verify(sshManager).getPair(eq(USER_ID), eq("workspace"), eq(WORKSPACE_ID));
verify(sshManager).getPairs(eq(OWNER), eq("machine"));
verify(sshManager).getPair(eq(OWNER), eq("workspace"), eq(WORKSPACE_ID));

verifyZeroInteractions(docker, environmentEngine, sshManager);
}

@Test
public void shouldSendMessageInMachineLoggerWhenSomeErrorOcursOnKeysInjection() throws Exception {
when(sshManager.getPairs(anyString(), anyString()))
.thenReturn(Collections.singletonList(new SshPairImpl(USER_ID, "machine", "myPair", "publicKey1", null)));
.thenReturn(Collections.singletonList(new SshPairImpl(OWNER, "machine", "myPair", "publicKey1", null)));
when(logMessage.getType()).thenReturn(LogMessage.Type.STDERR);
when(logMessage.getContent()).thenReturn("FAILED");

Expand Down

0 comments on commit 88a9a7e

Please sign in to comment.