Skip to content

Commit

Permalink
liveness information should also be available at /health (besides /he…
Browse files Browse the repository at this point in the history
…alth/liveness)

to avoid breaking changes
  • Loading branch information
asalan316 committed May 24, 2023
1 parent f09bc70 commit fbd69ea
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 29 deletions.
4 changes: 3 additions & 1 deletion jobs/golangapiserver/spec
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ properties:
Hash-Value of the password used for basic access authentication to connect to the protected health-endpoints.
More secure alternative of setting the password. Set to `""` if you don't want to use it.
autoscaler.apiserver.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.apiserver.health.readiness_enabled:
default: false
Expand Down
4 changes: 3 additions & 1 deletion jobs/metricsforwarder/spec
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ properties:
description: "the password of health endpoint"
default: ''
autoscaler.metricsforwarder.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.metricsforwarder.health.readiness_enabled:
default: false
Expand Down
4 changes: 3 additions & 1 deletion jobs/metricsgateway/spec
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ properties:
description: "the password of health endpoint"
default: ''
autoscaler.metricsgateway.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.metricsgateway.health.readiness_enabled:
default: false
Expand Down
4 changes: 3 additions & 1 deletion jobs/metricsserver/spec
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ properties:
description: "the password of health endpoint"
default: ''
autoscaler.metricsserver.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.metricsserver.health.readiness_enabled:
default: false
Expand Down
4 changes: 3 additions & 1 deletion jobs/operator/spec
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ properties:
description: "the password of health endpoint"
default: ''
autoscaler.operator.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.operator.health.readiness_enabled:
default: false
Expand Down
4 changes: 3 additions & 1 deletion jobs/scalingengine/spec
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ properties:
description: "the password of health endpoint"
default: ''
autoscaler.scalingengine.health.unprotected_endpoints:
description: "List of all health-endpoints, that run _without_ basic access authentication.Valid endpoints are /debug/pprof, /health/liveness, /health/prometheus, /health/readiness"
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /debug/pprof, /health, /health/liveness, /health/prometheus, /health/readiness
default: [] # protect everything
autoscaler.scalingengine.health.readiness_enabled:
default: false
Expand Down
2 changes: 1 addition & 1 deletion jobs/scheduler/spec
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ properties:
autoscaler.scheduler.health.unprotected_endpoints:
description: |
List of all health-endpoints, that run _without_ basic access authentication.
Valid endpoints are /health/liveness, /health/prometheus
Valid endpoints are /health, /health/liveness, /health/prometheus
default: [] # protect everything
autoscaler.changeloglock_timeout_seconds:
default: 180
Expand Down
44 changes: 31 additions & 13 deletions src/acceptance/api/basic_auth_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package api_test

import (
"fmt"
"net/http"
"strings"

Expand All @@ -10,8 +11,11 @@ import (

var _ = Describe("AutoScaler Health Endpoints with Basic Auth", func() {

urlfor := func(name string) func() string {
urlfor := func(name string, path string) func() string {
return func() string {
if path != "" {
healthURL = fmt.Sprintf("%s%s", cfg.ASApiEndpoint, path)
}
healthURL := strings.Replace(healthURL, cfg.ServiceName, cfg.ServiceName+"-"+name, 1)
return healthURL
}
Expand All @@ -20,12 +24,12 @@ var _ = Describe("AutoScaler Health Endpoints with Basic Auth", func() {
func(url func() string, statusCode func() int) {
Expect(Get(url())).To(Equal(statusCode()), "to get status code %d when getting %s", statusCode(), url())
},
Entry("API Server", urlfor("apiserver"), getStatus),
Entry("Eventgenerator", urlfor("eventgenerator"), getStatus),
Entry("Scaling Engine", urlfor("scalingengine"), getStatus),
Entry("Operator", urlfor("operator"), getStatus),
Entry("Metrics Forwarder", urlfor("metricsforwarder"), getStatus),
Entry("Scheduler", urlfor("scheduler"), getStatus),
Entry("API Server", urlfor("apiserver", ""), getStatus),
Entry("Eventgenerator", urlfor("eventgenerator", ""), getStatus),
Entry("Scaling Engine", urlfor("scalingengine", ""), getStatus),
Entry("Operator", urlfor("operator", ""), getStatus),
Entry("Metrics Forwarder", urlfor("metricsforwarder", ""), getStatus),
Entry("Scheduler", urlfor("scheduler", ""), getStatus),
)

DescribeTable("Basic Auth Credentials Provided",
Expand All @@ -34,12 +38,26 @@ var _ = Describe("AutoScaler Health Endpoints with Basic Auth", func() {
cfg.HealthEndpointsBasicAuthEnabled = true
Expect(Get(url())).To(Equal(statusCode()), "to get status code %d when getting %s", statusCode(), url())
},
Entry("API Server", urlfor("apiserver"), getStatus),
Entry("Eventgenerator", urlfor("eventgenerator"), getStatus),
Entry("Scaling Engine", urlfor("scalingengine"), getStatus),
Entry("Operator", urlfor("operator"), getStatus),
Entry("Metrics Forwarder", urlfor("metricsforwarder"), getStatus),
Entry("Scheduler", urlfor("scheduler"), getStatus),
Entry("API Server", urlfor("apiserver", ""), getStatus),
Entry("Eventgenerator", urlfor("eventgenerator", ""), getStatus),
Entry("Scaling Engine", urlfor("scalingengine", ""), getStatus),
Entry("Operator", urlfor("operator", ""), getStatus),
Entry("Metrics Forwarder", urlfor("metricsforwarder", ""), getStatus),
Entry("Scheduler", urlfor("scheduler", ""), getStatus),
)

DescribeTable("Liveness with Basic Auth Credentials Provided",

func(url func() string, statusCode func() int) {
cfg.HealthEndpointsBasicAuthEnabled = true
Expect(Get(url())).To(Equal(statusCode()), "to get status code %d when getting %s", statusCode(), url())
},
Entry("API Server", urlfor("apiserver", "/health"), getStatus),
Entry("Eventgenerator", urlfor("eventgenerator", "/health"), getStatus),
Entry("Scaling Engine", urlfor("scalingengine", "/health"), getStatus),
Entry("Operator", urlfor("operator", "/health"), getStatus),
Entry("Metrics Forwarder", urlfor("metricsforwarder", "/health"), getStatus),
Entry("Scheduler", urlfor("scheduler", "/health"), getStatus),
)
})

Expand Down
29 changes: 23 additions & 6 deletions src/autoscaler/healthendpoint/health_readiness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ var _ = Describe("Health Readiness", func() {
prometheus.NewRegistry(), func() time.Time { return *timesetter })
Expect(err).ShouldNot(HaveOccurred())
})

// TODO: Check that `NewHealthRouterWithBasicAuth` returns an error if and only if
// the configuration is bad, i.e.: protection needed but parameters for basic auth insufficient

Context("Authentication parameter checks", func() {
When("username and password are defined", func() {
BeforeEach(func() {
Expand Down Expand Up @@ -149,6 +145,18 @@ var _ = Describe("Health Readiness", func() {
End()
})
})
When("/health endpoint is called", func() {
It("should response with liveness", func() {
apitest.New().Debug().
Handler(healthRoute).
Get(routes.LivenessBasePath).
Expect(t).
Status(http.StatusOK).
Header("Content-Type", "application/json").
Body(`{"overall_status" : "UP", "checks" : [] }`).
End()
})
})
When("/health/readiness endpoint is called", func() {
It("should response OK", func() {
apitest.New().Debug().
Expand Down Expand Up @@ -362,6 +370,17 @@ var _ = Describe("Health Readiness", func() {
Body(`{"overall_status" : "UP", "checks" : [] }`).
End()
})
It("should have json response", func() {
apitest.New().
Handler(healthRoute).
Get(routes.LivenessBasePath).
BasicAuth("test-user-name", "test-user-password").
Expect(t).
Status(http.StatusOK).
Header("Content-Type", "application/json").
Body(`{"overall_status" : "UP", "checks" : [] }`).
End()
})
})

})
Expand All @@ -385,8 +404,6 @@ var _ = Describe("Health Readiness", func() {
End()
})
})
// TODO Q. The same endpoint, we would like to offer as with and without basic auth.
//look's complex
When("Default / endpoint is called", func() {
It("should require basic auth", func() {
apitest.New().
Expand Down
3 changes: 2 additions & 1 deletion src/autoscaler/healthendpoint/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func NewHealthRouterWithBasicAuth(conf models.HealthConfig, healthCheckers []Che
func addLivelinessHandlers(conf models.HealthConfig, mainRouter *mux.Router, time func() time.Time,
authMiddleware *basicAuthenticationMiddleware) error {
livenessHandler := common.VarsFunc(readiness([]Checker{}, time))
livenessRouter := mainRouter.PathPrefix(routes.LivenessPath).Subrouter()
livenessRouter := mainRouter.PathPrefix(routes.LivenessBasePath).Subrouter()

if endpointsNeedsProtection(routes.LivenessPath, conf) {
if !conf.BasicAuthPossible() {
Expand All @@ -118,6 +118,7 @@ func addLivelinessHandlers(conf models.HealthConfig, mainRouter *mux.Router, tim
livenessRouter.Use(authMiddleware.middleware)
}
livenessRouter.Handle("", livenessHandler)
livenessRouter.Handle("/liveness", livenessHandler)

return nil
}
Expand Down
3 changes: 1 addition & 2 deletions src/autoscaler/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ const (
SyncActiveSchedulesPath = "/v1/syncSchedules"
SyncActiveSchedulesRouteName = "SyncActiveSchedules"

BrokerHealthPath = "/health"

EnvelopePath = "/v1/envelopes"
EnvelopeReportRouteName = "ReportEnvelope"
CustomMetricsPath = "/v1/apps/{appid}/metrics"
Expand All @@ -56,6 +54,7 @@ const (
PublicApiInfoPath = "/v1/info"
PublicApiInfoRouteName = "GetPublicApiInfo"
LivenessPath = "/health/liveness"
LivenessBasePath = "/health"
ReadinessPath = "/health/readiness"
PrometheusPath = "/health/prometheus"
PprofPath = "/debug/pprof"
Expand Down

0 comments on commit fbd69ea

Please sign in to comment.