From e04da7d9b513758d2d10342f0a58035f14e8924f Mon Sep 17 00:00:00 2001 From: Eric Zimanyi Date: Mon, 24 Jun 2019 11:10:08 -0400 Subject: [PATCH] refactor(core): Use a rest controller for status endpoints (#388) * test(core): Update tests to test statusHandler The status endpoint tests are currently testing AllStatusEndpoint and InstanceStatusEndpoint which are pass-through classes that will be removed; update the tests to directly test StatusHandler. * refactor(core): Use a rest controller for status endpoints The Endpoint classes that are used to implement the status endpoint have significantly changed in Spring Boot 2. Instead of creating and exposing an endpoint, just use a rest controller. * fix(core): Remove package from componentscan The prior commit moved everything out of the endpoints package into other packages, so we should remove it from the component scan. --- .../com/netflix/spinnaker/rosco/Main.groovy | 1 - .../StatusController.groovy} | 20 +++++--- .../rosco/endpoints/AllStatusEndpoint.groovy | 42 ----------------- .../endpoints/InstanceStatusEndpoint.groovy | 44 ----------------- .../endpoints/mvc/AllStatusMvcEndpoint.groovy | 47 ------------------- .../mvc/InstanceStatusMvcEndpoint.groovy | 47 ------------------- .../AllStatusEndpointSpec.groovy | 40 ++++++---------- .../InstanceStatusEndpointSpec.groovy | 32 ++++++------- 8 files changed, 41 insertions(+), 232 deletions(-) rename rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/{endpoints/StatusHandler.groovy => controllers/StatusController.groovy} (79%) delete mode 100644 rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpoint.groovy delete mode 100644 rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpoint.groovy delete mode 100644 rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/AllStatusMvcEndpoint.groovy delete mode 100644 rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/InstanceStatusMvcEndpoint.groovy rename rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/{endpoints => controllers}/AllStatusEndpointSpec.groovy (76%) rename rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/{endpoints => controllers}/InstanceStatusEndpointSpec.groovy (73%) diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/Main.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/Main.groovy index fb0b7024f..7f6e2607b 100644 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/Main.groovy +++ b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/Main.groovy @@ -42,7 +42,6 @@ import javax.servlet.Filter @ComponentScan([ "com.netflix.spinnaker.rosco.config", "com.netflix.spinnaker.rosco.controllers", - "com.netflix.spinnaker.rosco.endpoints", "com.netflix.spinnaker.rosco.executor", "com.netflix.spinnaker.rosco.filters", "com.netflix.spinnaker.rosco.jobs", diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/StatusHandler.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/controllers/StatusController.groovy similarity index 79% rename from rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/StatusHandler.groovy rename to rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/controllers/StatusController.groovy index 46ea97c01..ac0afea56 100644 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/StatusHandler.groovy +++ b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/controllers/StatusController.groovy @@ -1,11 +1,11 @@ /* * Copyright 2016 Schibsted ASA. * - * Licensed under the Apache License, Version 2.0 (the "License"); + * 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 + * 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, @@ -14,15 +14,19 @@ * limitations under the License. */ -package com.netflix.spinnaker.rosco.endpoints +package com.netflix.spinnaker.rosco.controllers import com.netflix.spinnaker.rosco.api.BakeStatus import com.netflix.spinnaker.rosco.persistence.BakeStore import org.springframework.beans.factory.annotation.Autowired -import org.springframework.stereotype.Component +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RestController -@Component -class StatusHandler { +@ConditionalOnProperty(value = "endpoints.status.enabled", matchIfMissing = true) +@RestController +@RequestMapping("/status") +class StatusController { private final BakeStore bakeStore private final String roscoInstanceId @@ -32,16 +36,18 @@ class StatusHandler { } @Autowired - StatusHandler(BakeStore bakeStore, String roscoInstanceId) { + StatusController(BakeStore bakeStore, String roscoInstanceId) { this.bakeStore = bakeStore this.roscoInstanceId = roscoInstanceId } + @RequestMapping("/instance") public Map instanceIncompleteBakes() { def instanceIncompleteBakeIds = bakeStore.getThisInstanceIncompleteBakeIds() return getBakesAndInstanceStatus(instanceIncompleteBakeIds) } + @RequestMapping("/all") public Map allIncompleteBakes() { def instances = [:] def allIncompleteBakeIds = bakeStore.getAllIncompleteBakeIds() diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpoint.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpoint.groovy deleted file mode 100644 index 308d31d7b..000000000 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpoint.groovy +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2016 Schibsted ASA. - * - * 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.rosco.endpoints - -import groovy.util.logging.Slf4j -import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.actuate.endpoint.AbstractEndpoint -import org.springframework.boot.context.properties.ConfigurationProperties -import org.springframework.stereotype.Component - -@Component -@Slf4j -@ConfigurationProperties(prefix = "endpoints.status", ignoreUnknownFields = false) -class AllStatusEndpoint extends AbstractEndpoint> { - - private final StatusHandler statusHandler - - @Autowired - AllStatusEndpoint(StatusHandler statusHandler) { - super("status", false, true) - this.statusHandler = statusHandler - } - - @Override - public Map invoke() { - return statusHandler.allIncompleteBakes() - } -} diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpoint.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpoint.groovy deleted file mode 100644 index f99243688..000000000 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpoint.groovy +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2016 Schibsted ASA. - * - * 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.rosco.endpoints - -import groovy.util.logging.Slf4j -import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.actuate.endpoint.AbstractEndpoint -import org.springframework.boot.context.properties.ConfigurationProperties -import org.springframework.stereotype.Component - - -@Component -@Slf4j -@ConfigurationProperties(prefix = "endpoints.status", ignoreUnknownFields = false) -class InstanceStatusEndpoint extends AbstractEndpoint> { - - private final StatusHandler statusHandler - - @Autowired - InstanceStatusEndpoint(StatusHandler statusHandler) { - super("status", false, true) - this.statusHandler = statusHandler - } - - @Override - public Map invoke() { - return statusHandler.instanceIncompleteBakes() - } - -} diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/AllStatusMvcEndpoint.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/AllStatusMvcEndpoint.groovy deleted file mode 100644 index 6590ecf70..000000000 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/AllStatusMvcEndpoint.groovy +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2016 Schibsted ASA. - * - * 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.rosco.endpoints.mvc - -import com.netflix.spinnaker.rosco.endpoints.AllStatusEndpoint -import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter -import org.springframework.http.HttpStatus -import org.springframework.http.ResponseEntity -import org.springframework.stereotype.Component -import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RequestMethod -import org.springframework.web.bind.annotation.ResponseBody - -@Component -class AllStatusMvcEndpoint extends EndpointMvcAdapter { - - @Autowired - public AllStatusMvcEndpoint(AllStatusEndpoint delegate) { - super(delegate) - } - - @RequestMapping(value = '/all', method = RequestMethod.GET) - @ResponseBody - @Override - public Object invoke() { - if (!getDelegate().isEnabled()) { - return new ResponseEntity>(Collections.singletonMap( - "message", "This endpoint is disabled"), HttpStatus.NOT_FOUND) - } - return super.invoke() - } -} diff --git a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/InstanceStatusMvcEndpoint.groovy b/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/InstanceStatusMvcEndpoint.groovy deleted file mode 100644 index cbf55fd5c..000000000 --- a/rosco-web/src/main/groovy/com/netflix/spinnaker/rosco/endpoints/mvc/InstanceStatusMvcEndpoint.groovy +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2016 Schibsted ASA. - * - * 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.rosco.endpoints.mvc - -import com.netflix.spinnaker.rosco.endpoints.InstanceStatusEndpoint -import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter -import org.springframework.http.HttpStatus -import org.springframework.http.ResponseEntity -import org.springframework.stereotype.Component -import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RequestMethod -import org.springframework.web.bind.annotation.ResponseBody - -@Component -class InstanceStatusMvcEndpoint extends EndpointMvcAdapter { - - @Autowired - public InstanceStatusMvcEndpoint(InstanceStatusEndpoint delegate) { - super(delegate) - } - - @RequestMapping(value = '/instance', method = RequestMethod.GET) - @ResponseBody - @Override - public Object invoke() { - if (!getDelegate().isEnabled()) { - return new ResponseEntity>(Collections.singletonMap( - "message", "This endpoint is disabled"), HttpStatus.NOT_FOUND) - } - return super.invoke() - } -} diff --git a/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpointSpec.groovy b/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/AllStatusEndpointSpec.groovy similarity index 76% rename from rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpointSpec.groovy rename to rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/AllStatusEndpointSpec.groovy index ba5338e84..cb68978ea 100644 --- a/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/AllStatusEndpointSpec.groovy +++ b/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/AllStatusEndpointSpec.groovy @@ -1,11 +1,11 @@ /* * Copyright 2016 Schibsted ASA. * - * Licensed under the Apache License, Version 2.0 (the "License"); + * 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 + * 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, @@ -14,10 +14,11 @@ * limitations under the License. */ -package com.netflix.spinnaker.rosco.endpoints +package com.netflix.spinnaker.rosco.controllers import com.google.common.collect.Sets import com.netflix.spinnaker.rosco.api.BakeStatus +import com.netflix.spinnaker.rosco.controllers.StatusController import com.netflix.spinnaker.rosco.persistence.RedisBackedBakeStore import spock.lang.Specification import spock.lang.Subject @@ -34,13 +35,12 @@ class AllStatusEndpointSpec extends Specification { void 'all instances incomplete bakes with status'() { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) - def statusHandler = new StatusHandler(bakeStoreMock, localInstanceId) @Subject - def statusEndpoint = new AllStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, localInstanceId) when: - def instances = statusEndpoint.invoke() + def instances = statusHandler.allIncompleteBakes() then: 1 * bakeStoreMock.getAllIncompleteBakeIds() >> [(localInstanceId): Sets.newHashSet(LOCAL_JOB_ID), (remoteInstanceId): Sets.newHashSet(REMOTE_JOB_ID)] 1 * bakeStoreMock.retrieveBakeStatusById(LOCAL_JOB_ID) >> localRunningBakeStatus @@ -52,13 +52,10 @@ class AllStatusEndpointSpec extends Specification { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) bakeStoreMock.getAllIncompleteBakeIds() >> new HashMap>() - def statusHandler = new StatusHandler(bakeStoreMock, localInstanceId) - - @Subject - def statusEndpoint = new AllStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, localInstanceId) when: - def instances = statusEndpoint.invoke() + def instances = statusHandler.allIncompleteBakes() then: 1 * bakeStoreMock.getAllIncompleteBakeIds() @@ -68,13 +65,10 @@ class AllStatusEndpointSpec extends Specification { void 'no instances incomplete bakes'() { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) - def statusHandler = new StatusHandler(bakeStoreMock, localInstanceId) - - @Subject - def statusEndpoint = new AllStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, localInstanceId) when: - def instances = statusEndpoint.invoke() + def instances = statusHandler.allIncompleteBakes() then: instances == [instance: localInstanceId, instances: [:]] @@ -84,13 +78,10 @@ class AllStatusEndpointSpec extends Specification { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) bakeStoreMock.getAllIncompleteBakeIds() >> { throw new RuntimeException() } - def statusHandler = new StatusHandler(bakeStoreMock, localInstanceId) - - @Subject - def statusEndpoint = new AllStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, localInstanceId) when: - statusEndpoint.invoke() + statusHandler.allIncompleteBakes() then: thrown(RuntimeException) @@ -102,13 +93,10 @@ class AllStatusEndpointSpec extends Specification { bakeStoreMock.getAllIncompleteBakeIds() >> [(localInstanceId): Sets.newHashSet(LOCAL_JOB_ID), (remoteInstanceId): Sets.newHashSet(REMOTE_JOB_ID)] bakeStoreMock.retrieveBakeStatusById(LOCAL_JOB_ID) >> { throw new RuntimeException() } bakeStoreMock.retrieveBakeStatusById(REMOTE_JOB_ID) >> { throw new RuntimeException() } - def statusHandler = new StatusHandler(bakeStoreMock, localInstanceId) - - @Subject - def statusEndpoint = new AllStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, localInstanceId) when: - statusEndpoint.invoke() + statusHandler.allIncompleteBakes() then: thrown(RuntimeException) diff --git a/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpointSpec.groovy b/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/InstanceStatusEndpointSpec.groovy similarity index 73% rename from rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpointSpec.groovy rename to rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/InstanceStatusEndpointSpec.groovy index da050c3aa..545bab254 100644 --- a/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/endpoints/InstanceStatusEndpointSpec.groovy +++ b/rosco-web/src/test/groovy/com/netflix/spinnaker/rosco/controllers/InstanceStatusEndpointSpec.groovy @@ -1,11 +1,11 @@ /* * Copyright 2016 Schibsted ASA. * - * Licensed under the Apache License, Version 2.0 (the "License"); + * 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 + * 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, @@ -14,10 +14,11 @@ * limitations under the License. */ -package com.netflix.spinnaker.rosco.endpoints +package com.netflix.spinnaker.rosco.controllers import com.google.common.collect.Sets import com.netflix.spinnaker.rosco.api.BakeStatus +import com.netflix.spinnaker.rosco.controllers.StatusController import com.netflix.spinnaker.rosco.persistence.RedisBackedBakeStore import spock.lang.Specification import spock.lang.Subject @@ -32,13 +33,12 @@ class InstanceStatusEndpointSpec extends Specification { void 'instance incomplete bakes with status'() { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) - def statusHandler = new StatusHandler(bakeStoreMock, instanceId) @Subject - def statusEndpoint = new InstanceStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, instanceId) when: - def instanceInfo = statusEndpoint.invoke() + def instanceInfo = statusHandler.instanceIncompleteBakes() then: 1 * bakeStoreMock.getThisInstanceIncompleteBakeIds() >> Sets.newHashSet(JOB_ID) @@ -51,13 +51,12 @@ class InstanceStatusEndpointSpec extends Specification { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) bakeStoreMock.getThisInstanceIncompleteBakeIds() >> new HashSet<>() - def statusHandler = new StatusHandler(bakeStoreMock, instanceId) @Subject - def statusEndpoint = new InstanceStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, instanceId) when: - def instanceInfo = statusEndpoint.invoke() + def instanceInfo = statusHandler.instanceIncompleteBakes() then: 1 * bakeStoreMock.getThisInstanceIncompleteBakeIds() @@ -68,13 +67,12 @@ class InstanceStatusEndpointSpec extends Specification { void 'no instance incomplete bakes'() { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) - def statusHandler = new StatusHandler(bakeStoreMock, instanceId) @Subject - def statusEndpoint = new InstanceStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, instanceId) when: - def instanceInfo = statusEndpoint.invoke() + def instanceInfo = statusHandler.instanceIncompleteBakes() then: instanceInfo.bakes.isEmpty() @@ -85,13 +83,12 @@ class InstanceStatusEndpointSpec extends Specification { setup: def bakeStoreMock = Mock(RedisBackedBakeStore) bakeStoreMock.getThisInstanceIncompleteBakeIds() >> { throw new RuntimeException() } - def statusHandler = new StatusHandler(bakeStoreMock, instanceId) @Subject - def statusEndpoint = new InstanceStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, instanceId) when: - statusEndpoint.invoke() + statusHandler.instanceIncompleteBakes() then: thrown(RuntimeException) @@ -102,13 +99,12 @@ class InstanceStatusEndpointSpec extends Specification { def bakeStoreMock = Mock(RedisBackedBakeStore) bakeStoreMock.getThisInstanceIncompleteBakeIds() >> Sets.newHashSet(JOB_ID) bakeStoreMock.retrieveBakeStatusById(JOB_ID) >> { throw new RuntimeException() } - def statusHandler = new StatusHandler(bakeStoreMock, instanceId) @Subject - def statusEndpoint = new InstanceStatusEndpoint(statusHandler) + def statusHandler = new StatusController(bakeStoreMock, instanceId) when: - statusEndpoint.invoke() + statusHandler.instanceIncompleteBakes() then: thrown(RuntimeException)