diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilder.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilder.java index 84235d378c..f618b47b26 100644 --- a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilder.java +++ b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilder.java @@ -84,14 +84,30 @@ public PipelineBuilder withStage(String type) { } public PipelineBuilder withStages(List> stages) { - if (stages != null) { - stages.forEach( - it -> { - String type = it.remove("type").toString(); - String name = it.containsKey("name") ? it.remove("name").toString() : null; - withStage(type, name != null ? name : type, it); - }); + if (stages == null) { + throw new IllegalArgumentException( + "null stages in pipeline '" + + pipeline.getName() + + "' in application '" + + pipeline.getApplication() + + "'"); } + stages.forEach( + it -> { + String name = it.containsKey("name") ? it.remove("name").toString() : null; + if (!it.containsKey("type")) { + throw new IllegalArgumentException( + "type missing from pipeline '" + + pipeline.getName() + + "' in application '" + + pipeline.getApplication() + + "', stage name: '" + + name + + "'"); + } + String type = it.remove("type").toString(); + withStage(type, name != null ? name : type, it); + }); return this; } diff --git a/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilderTest.java b/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilderTest.java new file mode 100644 index 0000000000..e678d43636 --- /dev/null +++ b/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/model/PipelineBuilderTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2023 Salesforce, 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.netflix.spinnaker.orca.pipeline.model; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class PipelineBuilderTest { + + @Test + void withStagesChecksForNull() { + PipelineBuilder pipelineBuilder = new PipelineBuilder("my-application"); + assertThrows(IllegalArgumentException.class, () -> pipelineBuilder.withStages(null)); + } + + @Test + void withStagesChecksForType() { + PipelineBuilder pipelineBuilder = new PipelineBuilder("my-application"); + Map stageWithoutType = new HashMap<>(); + stageWithoutType.put("name", "my-pipeline-stage"); + assertThrows( + IllegalArgumentException.class, + () -> pipelineBuilder.withStages(List.of(stageWithoutType))); + } +} diff --git a/orca-web/src/test/java/com/netflix/spinnaker/orca/controllers/OperationsControllerTest.java b/orca-web/src/test/java/com/netflix/spinnaker/orca/controllers/OperationsControllerTest.java new file mode 100644 index 0000000000..d94450575f --- /dev/null +++ b/orca-web/src/test/java/com/netflix/spinnaker/orca/controllers/OperationsControllerTest.java @@ -0,0 +1,130 @@ +/* + * Copyright 2023 Salesforce, 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.netflix.spinnaker.orca.controllers; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.spinnaker.orca.Main; +import com.netflix.spinnaker.orca.notifications.NotificationClusterLock; +import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository; +import com.netflix.spinnaker.orca.q.pending.PendingExecutionService; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.web.context.WebApplicationContext; + +@SpringBootTest(classes = Main.class) +@TestPropertySource( + properties = { + "spring.config.location=classpath:orca-test.yml", + "keiko.queue.redis.enabled = false" + }) +class OperationsControllerTest { + + private MockMvc webAppMockMvc; + + @Autowired private WebApplicationContext webApplicationContext; + + @Autowired ObjectMapper objectMapper; + + @MockBean ExecutionRepository executionRepository; + + @MockBean PendingExecutionService pendingExecutionService; + + @MockBean NotificationClusterLock notificationClusterLock; + + @BeforeEach + void init(TestInfo testInfo) { + System.out.println("--------------- Test " + testInfo.getDisplayName()); + webAppMockMvc = webAppContextSetup(webApplicationContext).build(); + } + + @Test + void orchestrateWithEmptyRequestBody() throws Exception { + webAppMockMvc + .perform( + post("/orchestrate") + .contentType(MediaType.APPLICATION_JSON_VALUE) + .characterEncoding(StandardCharsets.UTF_8.toString())) + .andDo(print()) + .andExpect(status().isBadRequest()); + } + + @Test + void orchestrateWithEmptyPipelineMap() throws Exception { + webAppMockMvc + .perform( + post("/orchestrate") + .contentType(MediaType.APPLICATION_JSON_VALUE) + .characterEncoding(StandardCharsets.UTF_8.toString()) + .content(objectMapper.writeValueAsString(Map.of()))) + .andDo(print()) + // It's a little marginal to accept an empty pipeline since it's likely + // a user error to try to run a pipeline that doesn't do anything. + .andExpect(status().isOk()); + } + + @Test + void orchestrateWithoutStages() throws Exception { + Map pipelineWithoutStages = + Map.of("application", "my-application", "name", "my-pipeline-name"); + webAppMockMvc + .perform( + post("/orchestrate") + .contentType(MediaType.APPLICATION_JSON_VALUE) + .characterEncoding(StandardCharsets.UTF_8.toString()) + .content(objectMapper.writeValueAsString(pipelineWithoutStages))) + .andDo(print()) + // It's a little marginal to accept a pipeline without stages since it's + // likely a user error to try to run a pipeline that doesn't do + // anything. + .andExpect(status().isOk()); + } + + @Test + void orchestrateTreatsMissingTypeAsBadRequest() throws Exception { + Map pipelineWithStageWithoutType = + Map.of( + "application", + "my-application", + "name", + "my-pipeline-name", + "stages", + List.of(Map.of("name", "my-pipeline-stage-name"))); + webAppMockMvc + .perform( + post("/orchestrate") + .contentType(MediaType.APPLICATION_JSON_VALUE) + .characterEncoding(StandardCharsets.UTF_8.toString()) + .content(objectMapper.writeValueAsString(pipelineWithStageWithoutType))) + .andDo(print()) + .andExpect(status().isBadRequest()); + } +}