From 86201717ab0a59b109d88a4981955ff6871ba8e0 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Wed, 31 Jan 2024 13:52:49 -0800 Subject: [PATCH 1/8] WIP --- deploy-service/common/pom.xml | 4 +++ .../deployservice/bean/PromoteBean.java | 12 ++++++-- .../validation/CronExpressionConstraint.java | 27 +++++++++++++++++ .../CronExpressionConstraintValidator.java | 29 +++++++++++++++++++ deploy-service/pom.xml | 5 ++++ deploy-service/teletraanservice/pom.xml | 1 - .../teletraan/resource/EnvPromotes.java | 12 ++++---- .../teletraan/worker/AutoPromoter.java | 6 ++-- 8 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java create mode 100644 deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java diff --git a/deploy-service/common/pom.xml b/deploy-service/common/pom.xml index e7cc839493..8da0173be1 100644 --- a/deploy-service/common/pom.xml +++ b/deploy-service/common/pom.xml @@ -265,5 +265,9 @@ 0.10.5 runtime + + org.quartz-scheduler + quartz + \ No newline at end of file diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java index 3a77589944..7c941034b7 100644 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java @@ -4,9 +4,9 @@ * 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. @@ -16,10 +16,15 @@ package com.pinterest.deployservice.bean; import com.fasterxml.jackson.annotation.JsonProperty; +import com.pinterest.deployservice.validation.CronExpressionConstraint; + import org.apache.commons.lang.builder.ReflectionToStringBuilder; +import org.hibernate.validator.constraints.NotEmpty; import java.io.Serializable; +import javax.validation.constraints.NotNull; + /** * Keep the bean and table in sync *

@@ -39,6 +44,7 @@ */ public class PromoteBean implements Updatable, Serializable { @JsonProperty("envId") + @NotEmpty private String env_id; @JsonProperty("lastOperator") @@ -47,6 +53,7 @@ public class PromoteBean implements Updatable, Serializable { @JsonProperty("lastUpdate") private Long last_update; + @NotNull private PromoteType type; @JsonProperty("predStage") @@ -55,6 +62,7 @@ public class PromoteBean implements Updatable, Serializable { @JsonProperty("queueSize") private Integer queue_size; + @CronExpressionConstraint private String schedule; private Integer delay; diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java new file mode 100644 index 0000000000..c5a953b335 --- /dev/null +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java @@ -0,0 +1,27 @@ +package com.pinterest.deployservice.validation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import javax.validation.Constraint; +import javax.validation.Payload; + +@Constraint(validatedBy = { CronExpressionConstraintValidator.class }) +@Target({ + ElementType.METHOD, + ElementType.FIELD, + ElementType.ANNOTATION_TYPE, + ElementType.CONSTRUCTOR, + ElementType.PARAMETER, + ElementType.TYPE_USE +}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface CronExpressionConstraint { + String message() default "The given string is not an valid quartz cron expression."; + Class[] groups() default {}; + Class[] payload() default {}; +} diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java new file mode 100644 index 0000000000..683de42825 --- /dev/null +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java @@ -0,0 +1,29 @@ +package com.pinterest.deployservice.validation; + + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + +import org.quartz.CronExpression; + +public class CronExpressionConstraintValidator implements ConstraintValidator{ + + @Override + public void initialize(CronExpressionConstraint constraintAnnotation) { + } + + @Override + public boolean isValid(String schedule, ConstraintValidatorContext context) { + if (schedule == null) { + return true; + } + try { + new CronExpression(schedule); + return true; + } catch (Exception e) { + + return false; + } + } + +} diff --git a/deploy-service/pom.xml b/deploy-service/pom.xml index e182a8b7a8..3fda0f46d8 100644 --- a/deploy-service/pom.xml +++ b/deploy-service/pom.xml @@ -36,6 +36,11 @@ pom import + + org.quartz-scheduler + quartz + 2.3.2 + diff --git a/deploy-service/teletraanservice/pom.xml b/deploy-service/teletraanservice/pom.xml index bd91ba8026..f0ca139a9c 100644 --- a/deploy-service/teletraanservice/pom.xml +++ b/deploy-service/teletraanservice/pom.xml @@ -83,7 +83,6 @@ org.quartz-scheduler quartz - 2.3.2 diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvPromotes.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvPromotes.java index e6a7210aa1..1dfdf7c889 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvPromotes.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/resource/EnvPromotes.java @@ -4,9 +4,9 @@ * 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. @@ -74,10 +74,10 @@ public PromoteBean get(@ApiParam(value = "Environment name", required = true)@Pa value = "Update promote info", notes = "Updates promote info given environment and stage names by given promote info object") public void update(@Context SecurityContext sc, - @ApiParam(value = "Environment name", required = true)@PathParam("envName") String envName, - @ApiParam(value = "Stage name", required = true)@PathParam("stageName") String stageName, - @ApiParam(value = "Promote object to update with", required = true) - @Valid PromoteBean promoteBean) throws Exception { + @ApiParam(value = "Environment name", required = true) @PathParam("envName") String envName, + @ApiParam(value = "Stage name", required = true) @PathParam("stageName") String stageName, + @ApiParam(value = "Promote object to update with", required = true) @Valid PromoteBean promoteBean) + throws Exception { EnvironBean environBean = Utils.getEnvStage(environDAO, envName, stageName); authorizer.authorize(sc, new Resource(environBean.getEnv_name(), Resource.Type.ENV), Role.OPERATOR); String operator = sc.getUserPrincipal().getName(); diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/worker/AutoPromoter.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/worker/AutoPromoter.java index 595dc0dc00..b236da1420 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/worker/AutoPromoter.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/worker/AutoPromoter.java @@ -80,7 +80,7 @@ public AutoPromoter(ServiceContext serviceContext) { errorBudgetSuccess = Metrics.counter(MeterConstants.ERROR_BUDGET_METRIC_NAME, MeterConstants.ERROR_BUDGET_TAG_NAME_RESPONSE_TYPE, MeterConstants.ERROR_BUDGET_TAG_VALUE_RESPONSE_TYPE_SUCCESS, MeterConstants.ERROR_BUDGET_TAG_NAME_METHOD_NAME, this.getClass().getSimpleName()); - + errorBudgetFailure = Metrics.counter(MeterConstants.ERROR_BUDGET_METRIC_NAME, MeterConstants.ERROR_BUDGET_TAG_NAME_RESPONSE_TYPE, MeterConstants.ERROR_BUDGET_TAG_VALUE_RESPONSE_TYPE_FAILURE, MeterConstants.ERROR_BUDGET_TAG_NAME_METHOD_NAME, this.getClass().getSimpleName()); @@ -109,7 +109,7 @@ void processBatch() throws Exception { errorBudgetSuccess.increment(); } catch (Throwable t) { // Catch all throwable so that subsequent job not suppressed - LOG.error("AutoPromoter failed to process {}, Exception: {}", envId, t); + LOG.error("AutoPromoter failed to process {}", envId, t); errorBudgetFailure.increment(); } @@ -596,7 +596,7 @@ public void run() { } catch (Throwable t) { // Catch all throwable so that subsequent job not suppressed LOG.error("Failed to call AutoPromoter.", t); - + errorBudgetFailure.increment(); } } From 5311004fb613c921d6eae01517b1b83225690e60 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Wed, 31 Jan 2024 16:57:56 -0800 Subject: [PATCH 2/8] removed validator --- deploy-service/common/pom.xml | 5 +++ .../deployservice/bean/PromoteBean.java | 32 +++++++++++++++---- .../validation/CronExpressionConstraint.java | 27 ---------------- .../CronExpressionConstraintValidator.java | 29 ----------------- deploy-service/pom.xml | 1 + deploy-service/teletraanservice/pom.xml | 1 - 6 files changed, 31 insertions(+), 64 deletions(-) delete mode 100644 deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java delete mode 100644 deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java diff --git a/deploy-service/common/pom.xml b/deploy-service/common/pom.xml index 8da0173be1..9cd70bd3a2 100644 --- a/deploy-service/common/pom.xml +++ b/deploy-service/common/pom.xml @@ -269,5 +269,10 @@ org.quartz-scheduler quartz + + io.dropwizard + dropwizard-validation + ${dropwizard.version} + \ No newline at end of file diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java index 7c941034b7..ae5b2a9b19 100644 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java @@ -15,16 +15,20 @@ */ package com.pinterest.deployservice.bean; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.pinterest.deployservice.validation.CronExpressionConstraint; +import java.io.Serializable; + +import javax.validation.constraints.Max; +import javax.validation.constraints.Min; +import javax.validation.constraints.NotNull; import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.hibernate.validator.constraints.NotEmpty; +import org.quartz.CronExpression; -import java.io.Serializable; - -import javax.validation.constraints.NotNull; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.pinterest.deployservice.common.Constants; +import io.dropwizard.validation.ValidationMethod; /** * Keep the bean and table in sync *

@@ -44,7 +48,6 @@ */ public class PromoteBean implements Updatable, Serializable { @JsonProperty("envId") - @NotEmpty private String env_id; @JsonProperty("lastOperator") @@ -59,12 +62,13 @@ public class PromoteBean implements Updatable, Serializable { @JsonProperty("predStage") private String pred_stage; + @Min(1) @Max(Constants.DEFAULT_MAX_PROMOTE_QUEUE_SIZE) @JsonProperty("queueSize") private Integer queue_size; - @CronExpressionConstraint private String schedule; + @Min(0) private Integer delay; @JsonProperty("disablePolicy") @@ -173,4 +177,18 @@ public SetClause genSetClause() { public String toString() { return ReflectionToStringBuilder.toString(this); } + + @ValidationMethod(message = "schedule must be a valid cron expression") + public boolean isScheduleValid() { + if (schedule == null) { + return true; + } + try { + new CronExpression(schedule); + return true; + } catch (Exception e) { + + return false; + } + } } diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java deleted file mode 100644 index c5a953b335..0000000000 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraint.java +++ /dev/null @@ -1,27 +0,0 @@ -package com.pinterest.deployservice.validation; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import javax.validation.Constraint; -import javax.validation.Payload; - -@Constraint(validatedBy = { CronExpressionConstraintValidator.class }) -@Target({ - ElementType.METHOD, - ElementType.FIELD, - ElementType.ANNOTATION_TYPE, - ElementType.CONSTRUCTOR, - ElementType.PARAMETER, - ElementType.TYPE_USE -}) -@Retention(RetentionPolicy.RUNTIME) -@Documented -public @interface CronExpressionConstraint { - String message() default "The given string is not an valid quartz cron expression."; - Class[] groups() default {}; - Class[] payload() default {}; -} diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java deleted file mode 100644 index 683de42825..0000000000 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/validation/CronExpressionConstraintValidator.java +++ /dev/null @@ -1,29 +0,0 @@ -package com.pinterest.deployservice.validation; - - -import javax.validation.ConstraintValidator; -import javax.validation.ConstraintValidatorContext; - -import org.quartz.CronExpression; - -public class CronExpressionConstraintValidator implements ConstraintValidator{ - - @Override - public void initialize(CronExpressionConstraint constraintAnnotation) { - } - - @Override - public boolean isValid(String schedule, ConstraintValidatorContext context) { - if (schedule == null) { - return true; - } - try { - new CronExpression(schedule); - return true; - } catch (Exception e) { - - return false; - } - } - -} diff --git a/deploy-service/pom.xml b/deploy-service/pom.xml index 3fda0f46d8..3166f418e0 100644 --- a/deploy-service/pom.xml +++ b/deploy-service/pom.xml @@ -14,6 +14,7 @@ UTF-8 UTF-8 1.11.3 + 1.3.29 diff --git a/deploy-service/teletraanservice/pom.xml b/deploy-service/teletraanservice/pom.xml index f0ca139a9c..7a5641f155 100644 --- a/deploy-service/teletraanservice/pom.xml +++ b/deploy-service/teletraanservice/pom.xml @@ -19,7 +19,6 @@ UTF-8 UTF-8 true - 1.3.29 1.7.3 From c959ab51271a1b0af45aff494c07b3e15941adec Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 13:51:48 -0800 Subject: [PATCH 3/8] Making IllegalArgumentException a subclass of TeletraanException So it can be handled by existing handlers --- .../deploy_board/webapp/helpers/base_client.py | 2 +- .../deploy_board/webapp/helpers/exceptions.py | 13 ++++++++----- deploy-board/deploy_board/webapp/promote_views.py | 6 +++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/deploy-board/deploy_board/webapp/helpers/base_client.py b/deploy-board/deploy_board/webapp/helpers/base_client.py index 2b58269b57..1b670832af 100644 --- a/deploy-board/deploy_board/webapp/helpers/base_client.py +++ b/deploy-board/deploy_board/webapp/helpers/base_client.py @@ -63,7 +63,7 @@ def api(path, token=None, params=None, data=None): "Oops! You do not have the required permissions for this action. Contact an environment ADMIN for " "assistance. " + response.text) - if response.status_code == 400: + if response.status_code == 400 or response.status_code == 422: raise IllegalArgumentException(response.text) if response.status_code == 404: diff --git a/deploy-board/deploy_board/webapp/helpers/exceptions.py b/deploy-board/deploy_board/webapp/helpers/exceptions.py index 22ebb95304..4c0f518570 100644 --- a/deploy-board/deploy_board/webapp/helpers/exceptions.py +++ b/deploy-board/deploy_board/webapp/helpers/exceptions.py @@ -17,10 +17,6 @@ """ -class IllegalArgumentException(Exception): - pass - - class FailedAuthenticationException(Exception): pass @@ -34,4 +30,11 @@ class NotFoundException(Exception): class TeletraanException(Exception): - pass + def __init__(self, message: str, status=500) -> None: + super().__init__(message) + self.status = status + + +class IllegalArgumentException(TeletraanException): + def __init__(self, message: str) -> None: + super().__init__(message, status=400) diff --git a/deploy-board/deploy_board/webapp/promote_views.py b/deploy-board/deploy_board/webapp/promote_views.py index 2146f3d8bf..f3df822a24 100644 --- a/deploy-board/deploy_board/webapp/promote_views.py +++ b/deploy-board/deploy_board/webapp/promote_views.py @@ -23,6 +23,7 @@ from django.views.generic import View from . import common from .helpers import environs_helper +from .helpers.exceptions import TeletraanException, IllegalArgumentException class EnvPromoteConfigView(View): @@ -66,5 +67,8 @@ def post(self, request, name, stage): data["delay"] = int(query_dict["promoteDelay"]) if "promoteQueueSize" in query_dict: data["queueSize"] = int(query_dict["promoteQueueSize"]) - environs_helper.update_env_promotes_config(request, name, stage, data=data) + try: + environs_helper.update_env_promotes_config(request, name, stage, data=data) + except TeletraanException as e: + return HttpResponse(e, status=e.status, content_type="application/json") return self.get(request, name, stage) From e97bed672876c970527c52ccaeda94e27d92cd3e Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 15:02:07 -0800 Subject: [PATCH 4/8] Use Dropwizard validation exception mapper --- .../pinterest/deployservice/bean/PromoteBean.java | 7 ++++--- .../com/pinterest/teletraan/TeletraanService.java | 3 +++ .../teletraan/exception/GenericExceptionMapper.java | 13 ------------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java index ae5b2a9b19..74d128c173 100644 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java @@ -22,9 +22,9 @@ import javax.validation.constraints.NotNull; import org.apache.commons.lang.builder.ReflectionToStringBuilder; -import org.hibernate.validator.constraints.NotEmpty; import org.quartz.CronExpression; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.pinterest.deployservice.common.Constants; @@ -179,15 +179,16 @@ public String toString() { } @ValidationMethod(message = "schedule must be a valid cron expression") + @JsonIgnore public boolean isScheduleValid() { - if (schedule == null) { + if (type != PromoteType.AUTO) { return true; } + try { new CronExpression(schedule); return true; } catch (Exception e) { - return false; } } diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java index 0fda13f885..8105c93a86 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java @@ -24,6 +24,7 @@ import io.dropwizard.configuration.SubstitutingSourceProvider; import io.dropwizard.health.conf.HealthConfiguration; import io.dropwizard.health.core.HealthCheckBundle; +import io.dropwizard.jersey.validation.JerseyViolationExceptionMapper; import io.dropwizard.setup.Bootstrap; import io.dropwizard.setup.Environment; import io.swagger.jaxrs.config.BeanConfig; @@ -65,6 +66,8 @@ protected HealthConfiguration getHealthConfiguration(final TeletraanServiceConfi public void run(TeletraanServiceConfiguration configuration, Environment environment) throws Exception { TeletraanServiceContext context = ConfigHelper.setupContext(configuration); + environment.jersey().register(new JerseyViolationExceptionMapper()); + environment.jersey().register(configuration.getAuthenticationFactory().create(context)); Builds builds = new Builds(context); diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/exception/GenericExceptionMapper.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/exception/GenericExceptionMapper.java index 04512fd4dd..7369fd6392 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/exception/GenericExceptionMapper.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/exception/GenericExceptionMapper.java @@ -10,8 +10,6 @@ import java.io.StringWriter; import javax.servlet.http.HttpServletRequest; -import javax.validation.ConstraintViolationException; -import javax.validation.ConstraintViolation; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; @@ -56,17 +54,6 @@ public Response toResponse(Throwable t) { return Response.serverError().entity(sb.toString()).build(); } } - } else if (t instanceof ConstraintViolationException) { - StringBuilder sb = new StringBuilder(); - ConstraintViolationException cve = (ConstraintViolationException)t; - for (ConstraintViolation cv : cve.getConstraintViolations()) { - if (cv.getInvalidValue() != null) { - sb.append(cv.getPropertyPath().toString() + ":" + cv.getInvalidValue().toString()); - sb.append(" " + cv.getMessage()); - } - } - sb.append("\nParameters in request violate configured constraints."); - return Response.status(Response.Status.BAD_REQUEST).entity(sb.toString()).build(); } else { String errorMessage = buildErrorMessage(request); StringBuilder sb = new StringBuilder(); From 0e1f8d3255a6deb5cbfc12ec043605a87d760cf3 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 15:02:36 -0800 Subject: [PATCH 5/8] Better UI --- .../deploy_board/templates/configs/promote_config.tmpl | 2 +- deploy-board/deploy_board/templates/error.tmpl | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/deploy-board/deploy_board/templates/configs/promote_config.tmpl b/deploy-board/deploy_board/templates/configs/promote_config.tmpl index d20c39afe4..da7a04e687 100644 --- a/deploy-board/deploy_board/templates/configs/promote_config.tmpl +++ b/deploy-board/deploy_board/templates/configs/promote_config.tmpl @@ -235,7 +235,7 @@ btn.button('reset'); }, error: function (data) { - $('#errorBannerId').append(data.responseText); + $('#errorBannerId').append(`${data.status}: ${data.responseText}`); $('#errorBannerId').show(); } }); diff --git a/deploy-board/deploy_board/templates/error.tmpl b/deploy-board/deploy_board/templates/error.tmpl index cb62f669ec..c3097ba1c5 100644 --- a/deploy-board/deploy_board/templates/error.tmpl +++ b/deploy-board/deploy_board/templates/error.tmpl @@ -2,5 +2,9 @@

- + From 883327168e33f049e5d39fbd665d7d14fdb9e486 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 15:32:00 -0800 Subject: [PATCH 6/8] moved around --- .../java/com/pinterest/teletraan/TeletraanService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java index 8105c93a86..b612563379 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java @@ -24,6 +24,7 @@ import io.dropwizard.configuration.SubstitutingSourceProvider; import io.dropwizard.health.conf.HealthConfiguration; import io.dropwizard.health.core.HealthCheckBundle; +import io.dropwizard.jersey.jackson.JsonProcessingExceptionMapper; import io.dropwizard.jersey.validation.JerseyViolationExceptionMapper; import io.dropwizard.setup.Bootstrap; import io.dropwizard.setup.Environment; @@ -66,8 +67,6 @@ protected HealthConfiguration getHealthConfiguration(final TeletraanServiceConfi public void run(TeletraanServiceConfiguration configuration, Environment environment) throws Exception { TeletraanServiceContext context = ConfigHelper.setupContext(configuration); - environment.jersey().register(new JerseyViolationExceptionMapper()); - environment.jersey().register(configuration.getAuthenticationFactory().create(context)); Builds builds = new Builds(context); @@ -174,7 +173,9 @@ public void run(TeletraanServiceConfiguration configuration, Environment environ environment.healthChecks().register("generic", new GenericHealthCheck(context)); - // Exception handler + // Exception handlers + // Constrains validation exceptions, returns 4xx + environment.jersey().register(new JerseyViolationExceptionMapper()); environment.jersey().register(new GenericExceptionMapper(configuration.getSystemFactory().getClientError())); // Swagger API docs generation related From 7196829acfb05f041a51ea2b4f82f272c43208d9 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 16:07:33 -0800 Subject: [PATCH 7/8] Schedule can be blank --- .../java/com/pinterest/deployservice/bean/PromoteBean.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java index 74d128c173..869b21fd9a 100644 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/bean/PromoteBean.java @@ -22,6 +22,7 @@ import javax.validation.constraints.NotNull; import org.apache.commons.lang.builder.ReflectionToStringBuilder; +import org.apache.commons.lang3.StringUtils; import org.quartz.CronExpression; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -184,6 +185,9 @@ public boolean isScheduleValid() { if (type != PromoteType.AUTO) { return true; } + if (StringUtils.isBlank(schedule)) { + return true; + } try { new CronExpression(schedule); From 177235b381a703e82ae957222d1b879346f3c3e4 Mon Sep 17 00:00:00 2001 From: Tyler Ouyang Date: Thu, 1 Feb 2024 17:30:33 -0800 Subject: [PATCH 8/8] Add resources tests --- deploy-service/pom.xml | 27 ++++- deploy-service/teletraanservice/pom.xml | 12 ++ .../teletraan/resource/EnvPromotesTest.java | 105 ++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 deploy-service/teletraanservice/src/test/java/com/pinterest/teletraan/resource/EnvPromotesTest.java diff --git a/deploy-service/pom.xml b/deploy-service/pom.xml index 3166f418e0..12284f2a01 100644 --- a/deploy-service/pom.xml +++ b/deploy-service/pom.xml @@ -15,6 +15,7 @@ UTF-8 1.11.3 1.3.29 + 5.10.1 @@ -46,6 +47,19 @@ + + + org.junit.jupiter + junit-jupiter-engine + ${junit-jupiter.version} + test + + + + org.junit.vintage + junit-vintage-engine + ${junit-jupiter.version} + junit junit @@ -77,8 +91,19 @@ maven-source-plugin 2.1.2 + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + + org.apache.maven.plugins + maven-surefire-plugin + + @@ -122,4 +147,4 @@ - + \ No newline at end of file diff --git a/deploy-service/teletraanservice/pom.xml b/deploy-service/teletraanservice/pom.xml index 7a5641f155..dc0572de53 100644 --- a/deploy-service/teletraanservice/pom.xml +++ b/deploy-service/teletraanservice/pom.xml @@ -104,6 +104,18 @@ io.micrometer micrometer-core + + io.dropwizard + dropwizard-testing + ${dropwizard.version} + test + + + org.junit.jupiter + junit-jupiter-params + ${junit-jupiter.version} + test + diff --git a/deploy-service/teletraanservice/src/test/java/com/pinterest/teletraan/resource/EnvPromotesTest.java b/deploy-service/teletraanservice/src/test/java/com/pinterest/teletraan/resource/EnvPromotesTest.java new file mode 100644 index 0000000000..5622a9efcc --- /dev/null +++ b/deploy-service/teletraanservice/src/test/java/com/pinterest/teletraan/resource/EnvPromotesTest.java @@ -0,0 +1,105 @@ +package com.pinterest.teletraan.resource; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import java.util.stream.Stream; + +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.Response; + +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import com.pinterest.deployservice.bean.PromoteBean; +import com.pinterest.deployservice.bean.PromoteDisablePolicy; +import com.pinterest.deployservice.bean.PromoteType; +import com.pinterest.deployservice.common.Constants; +import com.pinterest.teletraan.TeletraanServiceContext; + +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import io.dropwizard.testing.junit5.ResourceExtension; + +@ExtendWith(DropwizardExtensionsSupport.class) +public class EnvPromotesTest { + + private static final ResourceExtension EXT; + + static { + TeletraanServiceContext context = new TeletraanServiceContext(); + + EXT = ResourceExtension.builder() + .addResource(new EnvPromotes(context)) + .build(); + } + + @ParameterizedTest + @MethodSource("invalidPromoteBeansSource") + public void invalidPromoteBeans_422(PromoteBean promoteBean) { + final Response put = EXT.target(Target.V1_ENV_PROMOTES).request().put(Entity.json(promoteBean)); + + assertEquals(422, put.getStatus()); + } + + @ParameterizedTest + @MethodSource("validPromoteBeansSource") + public void invalidPromoteBeans_not422(PromoteBean promoteBean) { + final Response put = EXT.target(Target.V1_ENV_PROMOTES).request().put(Entity.json(promoteBean)); + + assertNotEquals(422, put.getStatus()); + } + + static Stream invalidPromoteBeansSource() { + PromoteBean negativeQueueSize = validPromoteBean(); + negativeQueueSize.setQueue_size(-1); + PromoteBean largeQueueSize = validPromoteBean(); + largeQueueSize.setQueue_size(Constants.DEFAULT_MAX_PROMOTE_QUEUE_SIZE + 1); + PromoteBean negativeDelay = validPromoteBean(); + negativeDelay.setDelay(-1); + PromoteBean invalidCron = validPromoteBean(); + invalidCron.setSchedule("invalidCron"); + PromoteBean invalidCron2 = validPromoteBean(); + invalidCron2.setSchedule("0 0 0 * * *"); + + return Stream.of( + Arguments.of(new PromoteBean()), + Arguments.of(negativeQueueSize), + Arguments.of(largeQueueSize), + Arguments.of(negativeDelay), + Arguments.of(invalidCron), + Arguments.of(invalidCron2) + ); + } + + static Stream validPromoteBeansSource() { + PromoteBean manual = validPromoteBean(); + manual.setType(PromoteType.MANUAL); + manual.setSchedule("invalidCron"); + PromoteBean emptySchedule = validPromoteBean(); + emptySchedule.setSchedule(" "); + PromoteBean validSchedule = validPromoteBean(); + validSchedule.setSchedule("0 0 0 * * ?"); + + return Stream.of( + Arguments.of(validPromoteBean()), + Arguments.of(manual), + Arguments.of(validSchedule), + Arguments.of(emptySchedule) + ); + } + + static PromoteBean validPromoteBean() { + PromoteBean promoteBean = new PromoteBean(); + promoteBean.setQueue_size(1); + promoteBean.setDelay(0); + promoteBean.setType(PromoteType.AUTO); + promoteBean.setDisable_policy(PromoteDisablePolicy.AUTO); + return promoteBean; + } + + private static class Target { + private static final String V1_ENV_PROMOTES = "v1/envs/testEnv/testStage/promotes"; + } +}