UpdateVersion and deleteRule API for RampManager (#3159)

* UpdateVersion and deleteRule API for RampManager

Update Version for existing RampRule will update version metadata on DB directly; while delete rule will delete all metadata related to the rule. Both API would not introduce new insert but only update existing data.
Failure would occur when:
- no existing rule
- input failed validation
- no user permission

azkaban-common/src/main/java/azkaban/imagemgmt/daos/RampRuleDao.java

@@ -93,6 +93,16 @@ public interface RampRuleDao {
    */
   int addFlowDenyInfo(final List<ProjectFlow> flowIds, final String ruleName);
 
+  /**
+   * Update version on the rule, both table flow_deny_lists and ramp_rules got refreshed data.
+   *
+   * @param version - new version to be updated
+   * @param ruleName - ruleName in {@see ImageRampRule}
+   * @param user
+   * @throws azkaban.imagemgmt.exception.ImageMgmtDaoException
+   */
+  void updateVersionOnRule(final String version, final String ruleName, String user);
+
   enum DenyMode {
     ALL, // deny all versions, used for HP flow ramp rule
     PARTIAL // deny partial versions, versions need to be specified in normal ramp rule

azkaban-common/src/main/java/azkaban/imagemgmt/daos/RampRuleDaoImpl.java

@@ -20,6 +20,7 @@
 import azkaban.imagemgmt.dto.RampRuleFlowsDTO.ProjectFlow;
 import azkaban.imagemgmt.exception.ErrorCode;
 import azkaban.imagemgmt.exception.ImageMgmtDaoException;
+import azkaban.imagemgmt.exception.ImageMgmtInvalidInputException;
 import azkaban.imagemgmt.models.ImageRampRule;
 import azkaban.imagemgmt.models.RampRuleDenyList;
 import java.sql.ResultSet;
@@ -55,8 +56,19 @@ public class RampRuleDaoImpl implements RampRuleDao {
   private static final String UPDATE_RULE_OWNERSHIP = "UPDATE ramp_rules "
       + "SET owners=?, modified_by=?, modified_on=? WHERE rule_name=?";
 
+  private static final String UPDATE_VERSION_ON_RAMP_RULE = "UPDATE ramp_rules "
+      + "SET image_version=?, modified_by=?, modified_on=? WHERE rule_name=?";
+
+  private static final String DELETE_RULE_ON_RAMP_RULE = "DELETE from ramp_rules where rule_name = ?";
+
+  private static final String DELETE_FLOW_DENY_LIST = "DELETE from flow_deny_lists where rule_name = ?";
+
+  private static final String UPDATE_VERSION_ON_DENY_LIST = "UPDATE flow_deny_lists "
+      + "SET deny_version=? WHERE rule_name=?";
+
   private static final String INSERT_FLOW_DENY_LIST = "INSERT into flow_deny_lists "
       + "(flow_id, deny_mode, deny_version, rule_name) values (?, ?, ?, ?)";
+
   private static final String INSERT_HP_DENY_LIST = "INSERT into flow_deny_lists "
       + "(flow_id, deny_mode, rule_name) values (?, ?, ?)";
 
@@ -155,7 +167,7 @@ public ImageRampRule getRampRule(String ruleName) {
           new FetchRampRuleHandler(), ruleName);
       if (rampRule == null) {
         LOG.error("Can not find ramp rule at the ruleName: " + ruleName);
-        throw new ImageMgmtDaoException("ramp rule not found with ruleName: " + ruleName);
+        throw new ImageMgmtDaoException(ErrorCode.BAD_REQUEST, "ramp rule not found with ruleName: " + ruleName);
       }
       return rampRule;
     } catch (SQLException e) {
@@ -166,7 +178,18 @@ public ImageRampRule getRampRule(String ruleName) {
 
   @Override
   public void deleteRampRule(final String ruleName) {
-
+    final SQLTransaction<Long> deleteRampRuleAndFlowDenyList = transOperator -> {
+      transOperator.update(DELETE_RULE_ON_RAMP_RULE, ruleName);
+      transOperator.update(DELETE_FLOW_DENY_LIST, ruleName);
+      transOperator.getConnection().commit();
+      return transOperator.getLastInsertId();
+    };
+    try {
+      databaseOperator.transaction(deleteRampRuleAndFlowDenyList);
+    } catch (SQLException e) {
+      LOG.error("failed to delete ramp rule: " + ruleName);
+      throw new ImageMgmtDaoException("Error in deleting data based on ramp rule: " + ruleName);
+    }
   }
 
   /**
@@ -221,9 +244,8 @@ public int addFlowDenyInfo(final List<ProjectFlow> flowIds, final String ruleNam
       return transOperator.getLastInsertId();
     };
     // end SQL transaction operator
-    int batchInsertId = 0;
     try {
-      batchInsertId = this.databaseOperator.transaction(fetchRampRuleAndUpdateDenyList).intValue();
+      int batchInsertId = this.databaseOperator.transaction(fetchRampRuleAndUpdateDenyList).intValue();
       if (batchInsertId == 0) {
         LOG.warn(String.format("creating no new flow deny list based on rule: %s, "
                 + "flowList: %s. Might due to deny rule already exists", ruleName, flowIds));
@@ -245,6 +267,38 @@ public int addFlowDenyInfo(final List<ProjectFlow> flowIds, final String ruleNam
     }
   }
 
+  /**
+   * Update version on the rule, both table flow_deny_lists and ramp_rules got refreshed data.
+   * It will fetch ramp_rules to get metadata and operate UPDATE on both in a single transaction.
+   *
+   * @param newVersion
+   * @param ruleName - ruleName in {@see ImageRampRule}
+   * @param user
+   * @throws azkaban.imagemgmt.exception.ImageMgmtDaoException
+   */
+  @Override
+  public void updateVersionOnRule(final String newVersion, final String ruleName, final String user) {
+    final SQLTransaction<Long> fetchRampRuleAndUpdateDenyList = transOperator -> {
+      final ImageRampRule imageRampRule =
+          transOperator.query(FetchRampRuleHandler.FETCH_RAMP_RULE_BY_ID, new FetchRampRuleHandler(), ruleName);
+      final String newDenyVersion = String.join(":", imageRampRule.getImageName(), newVersion);
+      if (!newVersion.equals(imageRampRule.getImageVersion())) {
+        transOperator.update(UPDATE_VERSION_ON_RAMP_RULE,
+            newVersion, user, Timestamp.valueOf(LocalDateTime.now()), ruleName);
+        transOperator.update(UPDATE_VERSION_ON_DENY_LIST, newDenyVersion, ruleName);
+      }
+      transOperator.getConnection().commit();
+      return transOperator.getLastInsertId();
+    };
+    // end SQL transaction operator
+    try {
+      this.databaseOperator.transaction(fetchRampRuleAndUpdateDenyList);
+    } catch (final SQLException e) {
+      LOG.error("Unable to update the ramp rule metadata", e);
+      throw new ImageMgmtDaoException("Unable to update the ramp rule version: " + e.getMessage());
+    }
+  }
+
   public static class FetchRampRuleHandler implements ResultSetHandler<ImageRampRule> {
 
     private static final String FETCH_RAMP_RULE_BY_ID =

azkaban-web-server/src/main/java/azkaban/imagemgmt/services/ImageRampRuleService.java

@@ -18,6 +18,7 @@
 import azkaban.imagemgmt.dto.RampRuleFlowsDTO.ProjectFlow;
 import azkaban.imagemgmt.dto.RampRuleOwnershipDTO;
 import azkaban.imagemgmt.dto.ImageRampRuleRequestDTO;
+import azkaban.imagemgmt.exception.ImageMgmtDaoException;
 import azkaban.imagemgmt.exception.ImageMgmtException;
 import azkaban.imagemgmt.models.ImageRampRule;
 import azkaban.user.User;
@@ -64,7 +65,14 @@ public interface ImageRampRuleService {
   String updateRuleOwnership(final RampRuleOwnershipDTO RuleOwnershipDTO, final User user,
       final OperationType operationType);
 
-  void deleteRule(final String ruleName);
+  /**
+   * Delete ramp rule by given ruleName.
+   *
+   * @param ruleName - ruleName in {@link ImageRampRule}
+   * @param user - user must have the permission to delete
+   * @throws ImageMgmtDaoException - failures occur in DB transaction
+   * */
+  void deleteRule(final String ruleName, final User user);
 
   /**
    * add flows into ramp rules. Validation will be performed based on owner list, active project and valid flows.
@@ -76,7 +84,15 @@ String updateRuleOwnership(final RampRuleOwnershipDTO RuleOwnershipDTO, final Us
    * */
   void addFlowsToRule(final List<ProjectFlow> flowIds, final String ruleName, final User user);
 
-  void updateVersionOnRule(final String newVersion, final String ruleName);
+  /**
+   * Update normal ramp rule's version based on given ruleName.
+   *
+   * @param ruleName - ruleName in {@link ImageRampRule}
+   * @param newVersion- new version to be updated
+   * @param user - user must have the permission to operate
+   * @throws ImageMgmtException - failures on updating HPFlowRule or DB transaction
+   * */
+  void updateVersionOnRule(final String newVersion, final String ruleName, final User user);
 
   enum OperationType {
     ADD(),

azkaban-web-server/src/main/java/azkaban/imagemgmt/services/ImageRampRuleServiceImpl.java

@@ -87,7 +87,8 @@ public void createRule(final ImageRampRuleRequestDTO rampRuleRequest, final User
       .getImageTypeByName(rampRuleRequest.getImageName())
         .orElseThrow(() -> new ImageMgmtInvalidInputException(ErrorCode.NOT_FOUND, String.format("Unable to"
             + " fetch image type metadata. Invalid image type: %s.", rampRuleRequest.getImageName())));
-    if (!this.imageVersionDao.isInvalidVersion(rampRuleRequest.getImageName(), rampRuleRequest.getImageVersion())) {
+    if (this.imageVersionDao.isInvalidVersion(rampRuleRequest.getImageName(), rampRuleRequest.getImageVersion())) {
+      log.error("fail to validate image version: " + rampRuleRequest.getImageVersion());
       throw new ImageMgmtInvalidInputException(ErrorCode.NOT_FOUND, String.format(
           "Unable to fetch image version metadata. Invalid image version: %s.", rampRuleRequest.getImageVersion()));
     }
@@ -184,9 +185,22 @@ public String updateRuleOwnership(final RampRuleOwnershipDTO ruleOwnershipDTO, f
     return Strings.EMPTY;
   }
 
+  /**
+   * delete ramp rule's metadata based on given ruleName
+   *
+   * @param ruleName - ruleName in {@link ImageRampRule}
+   * @param user - user to operate
+   * */
   @Override
-  public void deleteRule(final String ruleName) {
-
+  public void deleteRule(final String ruleName, final User user) {
+    // validate permission
+    final Set<String> owners = rampRuleDao.getOwners(ruleName);
+    if (!permissionManager.hasPermission(user, owners)) {
+      log.error("current user "+ user.getUserId() + " does not have permission to delete ramp rule");
+      throw new ImageMgmtInvalidPermissionException(ErrorCode.UNAUTHORIZED,
+          "current user "+ user.getUserId() + " does not have permission to ramp rule");
+    }
+    rampRuleDao.deleteRampRule(ruleName);
   }
 
   /**
@@ -225,8 +239,34 @@ public void addFlowsToRule(final List<ProjectFlow> flowIds, final String ruleNam
     rampRuleDao.addFlowDenyInfo(flowIds, ruleName);
   }
 
+  /**
+   * Update normal ramp rule's version based on given ruleName, validated based on current user.
+   *
+   * @param ruleName - ruleName in {@link ImageRampRule}
+   * @param newVersion - new version to be updated
+   * @param user - user must have the permission to operate
+   * @throws ImageMgmtException
+   * */
   @Override
-  public void updateVersionOnRule(final String newVersion, final String ruleName) {
+  public void updateVersionOnRule(final String newVersion, final String ruleName, final User user) {
+    // validate permission
+    final ImageRampRule imageRampRule = rampRuleDao.getRampRule(ruleName);
+    final Set<String> owners = new HashSet<>(Arrays.asList(imageRampRule.getOwners().split(",")));
+    if (!permissionManager.hasPermission(user, owners)) {
+      log.error("current user "+ user.getUserId() + " does not have permission to add flows to Ramp rule");
+      throw new ImageMgmtInvalidPermissionException(ErrorCode.UNAUTHORIZED,
+          "current user "+ user.getUserId() + " does not have permission to add flows to Ramp rule");
+    }
+    if (imageRampRule.isHPRule()) {
+      log.error("Can't update version on a HP flow rule");
+      throw new ImageMgmtInvalidInputException(ErrorCode.BAD_REQUEST, "Can't update version on a HP flow rule");
+    }
+    if (this.imageVersionDao.isInvalidVersion(imageRampRule.getImageName(), newVersion)) {
+      log.error("fail to validate image version: " + newVersion);
+      throw new ImageMgmtInvalidInputException(ErrorCode.NOT_FOUND, String.format(
+          "Unable to fetch image version metadata. Invalid image version: %s.", newVersion));
+    }
 
+    rampRuleDao.updateVersionOnRule(newVersion, ruleName, user.getUserId());
   }
 }

azkaban-web-server/src/main/java/azkaban/imagemgmt/servlets/ImageRampRuleServlet.java

@@ -18,7 +18,9 @@
 import azkaban.imagemgmt.dto.RampRuleFlowsDTO;
 import azkaban.imagemgmt.dto.RampRuleOwnershipDTO;
 import azkaban.imagemgmt.dto.ImageRampRuleRequestDTO;
+import azkaban.imagemgmt.exception.ErrorCode;
 import azkaban.imagemgmt.exception.ImageMgmtException;
+import azkaban.imagemgmt.exception.ImageMgmtInvalidInputException;
 import azkaban.imagemgmt.services.ImageRampRuleService;
 import azkaban.imagemgmt.utils.ConverterUtils;
 import azkaban.server.HttpRequestUtils;
@@ -38,11 +40,11 @@
 
 /**
  * This servlet exposes the REST APIs such as create, delete and update rampup for image type.
- * Currently only supports:
+ * Current only supports:
  * Create Image Ramp Rule API: POST /imageRampRule/createRule
  * Create Image Ramp Rule API for HP flows: POST /imageRampRule/createHPFlowRule
  * Add managed flows into rule: POST /imageRampRule/addFlowsToRule
- * Modify image version on Rule: POST /imageRampRule/updateVersionOnRule
+ * Modify image version on Rule: POST /imageRampRule/updateVersion
  * Delete rule: POST /imageRampRule/deleteRule
  * Modify ownerships: POST /imageRampRule/addOwners or POST /imageRampRule/removeOwners
  */
@@ -52,11 +54,18 @@ public class ImageRampRuleServlet extends LoginAbstractAzkabanServlet {
   private ImageRampRuleService imageRampRuleService;
   private ConverterUtils utils;
 
+  /* URIs */
   private final static String CREATE_RULE_URI = "/imageRampRule/createRule";
   private final static String CREATE_HP_FLOW_RULE_URI = "/imageRampRule/createHPFlowRule";
   private final static String ADD_OWNERS_URI = "/imageRampRule/addOwners";
   private final static String REMOVE_OWNERS_URI = "/imageRampRule/removeOwners";
   private final static String ADD_FLOWS_TO_RULE_URI = "/imageRampRule/addFlowsToRule";
+  private final static String UPDATE_VERSION_ON_RULE_URI = "/imageRampRule/updateVersion";
+  private final static String DELETE_RULE_URI = "/imageRampRule/deleteRule";
+
+  /* Params */
+  private final static String RULE_NAME = "ruleName";
+  private final static String VERSION = "version";
 
   public ImageRampRuleServlet() {
     super(new ArrayList<>());
@@ -98,6 +107,12 @@ protected void handlePost(HttpServletRequest req, HttpServletResponse resp, Sess
         case ADD_FLOWS_TO_RULE_URI:
           handleAddFlowsToRule(req, resp, user);
           break;
+        case UPDATE_VERSION_ON_RULE_URI:
+          handleUpdateVersionOnRule(req, resp, user);
+          break;
+        case DELETE_RULE_URI:
+          handleDeleteRule(req, resp, user);
+          break;
       }
 
   }
@@ -202,4 +217,60 @@ private void handleAddFlowsToRule(final HttpServletRequest req,
       resp.setStatus(e.getErrorCode().getCode(), e.getMessage());
     }
   }
+
+  /**
+   * Updates a ramp rule's version based on given ruleName and version in parameters.
+   * Successful call would return OK(200).
+   *
+   * @throws ImageMgmtException with different ErrorCode, and the detailed error message.
+   **/
+  private void handleUpdateVersionOnRule(final HttpServletRequest req,
+                                         final HttpServletResponse resp,
+                                         final User user) {
+    try {
+      final String ruleName = getReqParam(req, RULE_NAME);
+      final String version = getReqParam(req, VERSION);
+      imageRampRuleService.updateVersionOnRule(version, ruleName, user);
+      resp.setStatus(HttpServletResponse.SC_OK);
+    } catch (ImageMgmtException e) {
+      LOG.error("fail to update version by rule " + req);
+      resp.setStatus(e.getErrorCode().getCode(), e.getMessage());
+    }
+  }
+
+  /**
+   * Delete a ramp rule.
+   * Successful call would return OK(200).
+   *
+   * @throws ImageMgmtException with different ErrorCode, and the detailed error message.
+   **/
+  private void handleDeleteRule(final HttpServletRequest req,
+                                final HttpServletResponse resp,
+                                final User user) {
+    try {
+      final String ruleName = getReqParam(req, RULE_NAME);
+      imageRampRuleService.deleteRule(ruleName, user);
+      resp.setStatus(HttpServletResponse.SC_OK);
+    } catch (ImageMgmtException e) {
+      LOG.error("fail to delete rule " + req);
+      resp.setStatus(e.getErrorCode().getCode(), e.getMessage());
+    }
+  }
+
+  /**
+   * Fetch RuleName parameter from request.
+   *
+   * @param req - Http request {@link HttpServletRequest}
+   * @return targetParam values
+   * @throws ImageMgmtInvalidInputException if not found required param
+   * */
+  private String getReqParam(final HttpServletRequest req, final String requiredParam) {
+    final String ruleName = req.getParameter(requiredParam);
+    if (ruleName == null || ruleName.length() == 0) {
+      LOG.error("{} must not be null or empty", requiredParam);
+      throw new ImageMgmtInvalidInputException(ErrorCode.BAD_REQUEST, requiredParam +" must not be null or empty");
+    }
+    return ruleName;
+  }
+
 }