Skip to content
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

Jump to bottom

Add authz to hosts endpoints #1617

Merged
merged 1 commit into from
May 25, 2024
Merged

Add authz to hosts endpoints #1617

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
13 changes: 13 additions & 0 deletions deploy-service/common/src/main/java/com/pinterest/deployservice/dao/EnvironDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ public interface EnvironDAO {

List<EnvironBean> getEnvsByHost(String host) throws Exception;

/**
* Retrieves the main environment ID for the specified host ID.
*
* <p>The main environment is where the cluster that the host belongs to is created.
* In case such an environment does not exist, the method will attempt to retrieve the
* environment that matches the first group that's known to Teletraan for the specified host.
*
* @param hostId The ID of the host.
* @return The bean represents the main environment for the specified host ID.
* @throws SQLException if an error occurs while retrieving the main environment ID.
*/
EnvironBean getMainEnvByHostId(String hostId) throws SQLException;

List<EnvironBean> getEnvsByGroups(Collection<String> groups) throws Exception;

List<String> getCurrentDeployIds() throws Exception;
Expand Down
20 changes: 20 additions & 0 deletions deploy-service/common/src/main/java/com/pinterest/deployservice/db/DBEnvironDAOImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ public class DBEnvironDAOImpl implements EnvironDAO {
private static final String GET_ENV_BY_CONSTRAINT_ID = "SELECT * FROM environs WHERE deploy_constraint_id = ?";
private static final String DELETE_DEPLOY_CONSTRAINT =
"UPDATE environs SET deploy_constraint_id=null WHERE env_name=? AND stage_name=?";
private static final String GET_MAIN_ENV_BY_HOST_ID = "SELECT e.* " +
"FROM hosts_and_agents ha " +
"JOIN environs e ON ha.auto_scaling_group = e.cluster_name " +
"WHERE ha.host_id = ? " +
"UNION " +
"(SELECT e.* " +
"FROM hosts h " +
"JOIN environs e ON h.group_name = e.cluster_name " +
"WHERE h.host_id = ? AND NOT EXISTS ( " +
" SELECT 1 " +
" FROM hosts_and_agents ha " +
" JOIN environs e ON ha.auto_scaling_group = e.cluster_name " +
" WHERE ha.host_id = ? )" +
"ORDER BY h.create_date ASC LIMIT 1)";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a LIMIT 1 on the outer query as well? or do extra rows just get ignored so it does not matter?

Also, I'm thinking the subquery could be removed if we had an outer LIMIT 1 and the first select was ordered before the second select. Might not make much difference.

LGTM overall

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The NOT EXISTS clause that checks whether the first query returns any results. If the first query returns results, the NOT EXISTS clause will be false, and the second query will not return any results. If the first query does not return any results, the NOT EXISTS clause will be true, and the second query will return its results.

We use BeanHandler so it will take the first result. Although we don't have unique key constraint on the cluster_name column, we don't have meaningful duplications

 select  cluster_name from environs GROUP BY cluster_name having count(*) > 1;
+--------------+
| cluster_name |
+--------------+
| NULL         |
|              |
+--------------+
2 rows in set (0.00 sec)


private BasicDataSource dataSource;

Expand Down Expand Up @@ -250,6 +264,12 @@ public List<EnvironBean> getEnvsByHost(String host) throws Exception {
return new ArrayList<EnvironBean>(envSet);
}

@Override
public EnvironBean getMainEnvByHostId(String hostId) throws SQLException {
ResultSetHandler<EnvironBean> h = new BeanHandler<>(EnvironBean.class);
return new QueryRunner(dataSource).query(GET_MAIN_ENV_BY_HOST_ID, h, hostId, hostId, hostId);
}

@Override
public List<EnvironBean> getEnvsByGroups(Collection<String> groups) throws Exception {
ResultSetHandler<List<EnvironBean>> h = new BeanListHandler<>(EnvironBean.class);
Expand Down
26 changes: 25 additions & 1 deletion deploy-service/common/src/main/java/com/pinterest/deployservice/handler/EnvironHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,18 @@
import com.pinterest.deployservice.dao.PromoteDAO;
import com.pinterest.deployservice.dao.ScheduleDAO;

import com.pinterest.deployservice.bean.ScheduleState;

import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.sql.SQLException;
import java.util.*;

import javax.ws.rs.NotAllowedException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.WebApplicationException;

public class EnvironHandler {
private static final Logger LOG = LoggerFactory.getLogger(EnvironHandler.class);

Expand Down Expand Up @@ -522,4 +526,24 @@ public void stopServiceOnHosts(Collection<String> hostIds, boolean replaceHost)
stopServiceOnHost(hostId, replaceHost);
}
}

public void ensureHostsOwnedByEnv(EnvironBean environBean, Collection<String> hostIds)
throws WebApplicationException {
for (String hostId : hostIds) {
try {
EnvironBean mainEnv = environDAO.getMainEnvByHostId(hostId);
if (mainEnv == null) {
throw new NotFoundException(
String.format("No main environment found for host %s, refuse to proceed", hostId));
}
if (!mainEnv.getEnv_id().equals(environBean.getEnv_id())) {
throw new NotAllowedException(String.format("%s/%s is not the owning environment of host %s",
environBean.getEnv_name(), environBean.getStage_name(), hostId));
}
} catch (SQLException e) {
throw new WebApplicationException(String.format("Failed to get main environment for host %s", hostId),
e);
}
}
}
}
125 changes: 125 additions & 0 deletions deploy-service/common/src/test/java/com/pinterest/deployservice/db/DBEnvironDAOImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/**
* Copyright (c) 2024 Pinterest, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.pinterest.deployservice.db;

import static org.junit.Assert.assertNull;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.time.Instant;

import com.pinterest.deployservice.bean.BeanUtils;
import com.pinterest.deployservice.bean.EnvironBean;
import com.pinterest.deployservice.bean.HostAgentBean;
import com.pinterest.deployservice.bean.HostBean;
import com.pinterest.deployservice.dao.EnvironDAO;
import com.pinterest.deployservice.dao.HostAgentDAO;
import com.pinterest.deployservice.dao.HostDAO;
import com.pinterest.deployservice.fixture.EnvironBeanFixture;
import org.apache.commons.dbcp.BasicDataSource;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class DBEnvironDAOImplTest {
private static final String HOST_ID = "host123";
private static final String TEST_CLUSTER = "test-cluster";
private static BasicDataSource dataSource;
private static HostAgentDAO hostAgentDAO;
private static HostDAO hostDAO;
private EnvironDAO sut;

@BeforeAll
static void setUpAll() throws Exception {
dataSource = DBUtils.createTestDataSource();
hostAgentDAO = new DBHostAgentDAOImpl(dataSource);
hostDAO = new DBHostDAOImpl(dataSource);
}

@BeforeEach
void setUp() {
sut = new DBEnvironDAOImpl(dataSource);
}

@AfterEach
void tearDown() throws Exception {
DBUtils.truncateAllTables(dataSource);
}

@Test
void testGetMainEnvByHostId_happyPath() throws Exception {
EnvironBean expectedEnvBean = EnvironBeanFixture.createRandomEnvironBean();
expectedEnvBean.setCluster_name(TEST_CLUSTER);
sut.insert(expectedEnvBean);

HostAgentBean hostAgentBean = new HostAgentBean();
hostAgentBean.setHost_id(HOST_ID);
hostAgentBean.setAuto_scaling_group(TEST_CLUSTER);
hostAgentDAO.insert(hostAgentBean);

HostBean hostBean = BeanUtils.createHostBean(Instant.now());
hostBean.setHost_id(HOST_ID);
hostBean.setGroup_name(TEST_CLUSTER + "sidecar");
hostDAO.insert(hostBean);

EnvironBean actualEnvironBean = sut.getMainEnvByHostId(HOST_ID);
assertEquals(expectedEnvBean.getEnv_name(), actualEnvironBean.getEnv_name());
assertEquals(expectedEnvBean.getStage_name(), actualEnvironBean.getStage_name());
assertEquals(TEST_CLUSTER, actualEnvironBean.getCluster_name());

EnvironBean nullEnvironBean = sut.getMainEnvByHostId("random-host-id");
assertNull(nullEnvironBean);
}

@Test
void testGetMainEnvByHostId_noHost() throws Exception {
EnvironBean actualEnvironBean = sut.getMainEnvByHostId(HOST_ID);
assertNull(actualEnvironBean);
}

@Test
void testGetMainEnvByHostId_noEnv() throws Exception {
HostAgentBean hostAgentBean = new HostAgentBean();
hostAgentBean.setHost_id(HOST_ID);
hostAgentBean.setAuto_scaling_group(TEST_CLUSTER);
hostAgentDAO.insert(hostAgentBean);

EnvironBean actualEnvironBean = sut.getMainEnvByHostId(HOST_ID);
assertNull(actualEnvironBean);
}

@Test
void testGetMainEnvByHostId_noHostAgent() throws Exception {
EnvironBean expectedEnvBean = EnvironBeanFixture.createRandomEnvironBean();
expectedEnvBean.setCluster_name(TEST_CLUSTER);
sut.insert(expectedEnvBean);

HostBean hostBean = BeanUtils.createHostBean(Instant.now());
hostBean.setHost_id(HOST_ID);
hostBean.setGroup_name(TEST_CLUSTER);
hostDAO.insert(hostBean);

HostBean hostBean2 = BeanUtils.createHostBean(Instant.now());
hostBean.setHost_id(HOST_ID);
hostBean.setGroup_name(TEST_CLUSTER + "2");
hostDAO.insert(hostBean2);

EnvironBean actualEnvironBean = sut.getMainEnvByHostId(HOST_ID);
assertEquals(expectedEnvBean.getEnv_name(), actualEnvironBean.getEnv_name());
assertEquals(expectedEnvBean.getStage_name(), actualEnvironBean.getStage_name());
assertEquals(TEST_CLUSTER, actualEnvironBean.getCluster_name());
}
}
58 changes: 49 additions & 9 deletions ...-service/common/src/test/java/com/pinterest/deployservice/handler/EnvironHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
package com.pinterest.deployservice.handler;

import static org.junit.Assert.assertEquals;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.junit.Before;
import org.junit.Test;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.List;

import javax.ws.rs.NotAllowedException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.WebApplicationException;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;

import com.pinterest.deployservice.ServiceContext;
Expand All @@ -18,14 +30,15 @@
import com.pinterest.deployservice.dao.EnvironDAO;
import com.pinterest.deployservice.dao.HostDAO;

public class EnvironHandlerTest {
class EnvironHandlerTest {
private final static String DEFAULT_HOST_ID = "hostId";

private EnvironHandler environHandler;

private HostDAO mockHostDAO;
private AgentDAO mockAgentDAO;
private EnvironDAO environDAO;
private List<String> hostIds = Arrays.asList("hostId1", "hostId2");

private ServiceContext createMockServiceContext() throws Exception {
mockHostDAO = mock(HostDAO.class);
Expand All @@ -39,13 +52,13 @@ private ServiceContext createMockServiceContext() throws Exception {
return serviceContext;
}

@Before
public void setUp() throws Exception {
@BeforeEach
void setUp() throws Exception {
environHandler = new EnvironHandler(createMockServiceContext());
}

@Test
public void stopServiceOnHost_withReplaceHost_hostBeanStateIsPendingTerminate() throws Exception {
void stopServiceOnHost_withReplaceHost_hostBeanStateIsPendingTerminate() throws Exception {
ArgumentCaptor<HostBean> argument = ArgumentCaptor.forClass(HostBean.class);

environHandler.stopServiceOnHost(DEFAULT_HOST_ID, true);
Expand All @@ -54,7 +67,7 @@ public void stopServiceOnHost_withReplaceHost_hostBeanStateIsPendingTerminate()
}

@Test
public void stopServiceOnHost_withoutReplaceHost_hostBeanStateIsPendingTerminateNoReplace() throws Exception {
void stopServiceOnHost_withoutReplaceHost_hostBeanStateIsPendingTerminateNoReplace() throws Exception {
ArgumentCaptor<HostBean> argument = ArgumentCaptor.forClass(HostBean.class);

environHandler.stopServiceOnHost(DEFAULT_HOST_ID, false);
Expand All @@ -63,12 +76,39 @@ public void stopServiceOnHost_withoutReplaceHost_hostBeanStateIsPendingTerminate
}

@Test
public void updateStage_type_enables_private_build() throws Exception {
void updateStage_type_enables_private_build() throws Exception {
ArgumentCaptor<EnvironBean> argument = ArgumentCaptor.forClass(EnvironBean.class);
EnvironBean envBean = new EnvironBean();
envBean.setStage_type(EnvType.DEV);
environHandler.createEnvStage(envBean, "Anonymous");
verify(environDAO).insert(argument.capture());
assertEquals(true, argument.getValue().getAllow_private_build());
}

@Test
void ensureHostsOwnedByEnv_noMainEnv() throws Exception {
assertThrows(NotFoundException.class, () -> environHandler.ensureHostsOwnedByEnv(new EnvironBean(), hostIds));
}

@Test
void ensureHostsOwnedByEnv_differentMainEnv() throws Exception {
EnvironBean envBean = new EnvironBean();
envBean.setEnv_id("envId");
when(environDAO.getMainEnvByHostId(anyString())).thenReturn(envBean);
assertThrows(NotAllowedException.class, () -> environHandler.ensureHostsOwnedByEnv(new EnvironBean(), hostIds));
}

@Test
void ensureHostsOwnedByEnv_sameMainEnv() throws Exception {
EnvironBean envBean = new EnvironBean();
envBean.setEnv_id("envId");
when(environDAO.getMainEnvByHostId(anyString())).thenReturn(envBean);
assertDoesNotThrow(() -> environHandler.ensureHostsOwnedByEnv(envBean, hostIds));
}

@Test
void ensureHostsOwnedByEnv_sqlException() throws Exception {
when(environDAO.getMainEnvByHostId(anyString())).thenThrow(SQLException.class);
assertThrows(WebApplicationException.class, () -> environHandler.ensureHostsOwnedByEnv(new EnvironBean(), hostIds));
}
}
4 changes: 2 additions & 2 deletions ...oy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvAgents.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public AgentErrorBean getAgentError(
value = "Update host agent",
notes = "Updates host agent specified by given environment name, stage name, and host id with given " +
"agent object")
@RolesAllowed(TeletraanPrincipalRole.Names.WRITE)
@RolesAllowed(TeletraanPrincipalRole.Names.EXECUTE)
@ResourceAuthZInfo(type = AuthZResource.Type.ENV_STAGE, idLocation = ResourceAuthZInfo.Location.PATH)
public void update(
@Context SecurityContext sc,
Expand All @@ -118,7 +118,7 @@ public void update(
@ApiOperation(
value = "Reset failed deploys",
notes = "Resets failing deploys given an environment name, stage name, and deploy id")
@RolesAllowed(TeletraanPrincipalRole.Names.WRITE)
@RolesAllowed(TeletraanPrincipalRole.Names.EXECUTE)
@ResourceAuthZInfo(type = AuthZResource.Type.ENV_STAGE, idLocation = ResourceAuthZInfo.Location.PATH)
public void resetFailedDeploys(
@Context SecurityContext sc,
Expand Down
1 change: 0 additions & 1 deletion ...y-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvDeploys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ public Response action(
response = Response.class)
@RolesAllowed(TeletraanPrincipalRole.Names.EXECUTE)
@ResourceAuthZInfo(type = AuthZResource.Type.ENV_STAGE, idLocation = ResourceAuthZInfo.Location.PATH)
// TODO: sidecar owners can perform actions on non-sidecar agents
public void update(
@Context SecurityContext sc,
@ApiParam(value = "Environment name", required = true)@PathParam("envName") String envName,
Expand Down
Loading
Loading