Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor v1 HTTP to use middlewares and http.HandlerFuncs #325

Merged
merged 30 commits into from
Apr 21, 2020
Merged

Refactor v1 HTTP to use middlewares and http.HandlerFuncs #325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
cdcc68d
pkg/store/ratelimited: Create HTTP middleware
metalmatze Mar 30, 2020
b237e41
pkg/http/server: Refactor to be Post to be HandleFunc, create Snappy …
metalmatze Mar 30, 2020
75c27ea
pkg/validate: Create PartitionKey middleware
metalmatze Mar 30, 2020
30f6c1a
Use new middlewares and use chi Router as HTTP mux
metalmatze Mar 30, 2020
ad1a1ee
Replace server.Post handler with foward.Handler entirely
metalmatze Mar 30, 2020
1924379
Fix some staticcheck issues
metalmatze Mar 31, 2020
5bf077f
pkg/http/server: Delete old and now unused Post handler
metalmatze Mar 31, 2020
1d08da2
Delete unused store.Store interface
metalmatze Mar 31, 2020
b42fb6a
pkg/store/memstore: Delete unused memstore
metalmatze Mar 31, 2020
7885e6d
Start moving HTTP API v1 files into pkg/server
metalmatze Mar 31, 2020
2e1f41a
Consolidate Telemeter server middlewares and handlers into pkg/server
metalmatze Mar 31, 2020
0f78d48
Refactor Validator to HTTP middleware
metalmatze Apr 3, 2020
fa52d2a
pkg/server: Add GoDoc comments to public functions
metalmatze Apr 3, 2020
a53a7ad
Use server.Validate middleware in Telemeter server
metalmatze Apr 3, 2020
4db0156
Address comments about refactoring clean ups
metalmatze Apr 6, 2020
a284906
Rename PartitionKey to ClusterIDKey and pass around clusterID
metalmatze Apr 6, 2020
7b6b0da
Update vendor/ folder to add missing go-chi/chi router
metalmatze Apr 6, 2020
7696a7c
pkg/server: Fix reader assignment in snappy.go
metalmatze Apr 6, 2020
de659a9
Use ClusterID middleware in v1 handler
metalmatze Apr 15, 2020
a9d3ab5
Improve validator middleware to correctly forward http body
metalmatze Apr 15, 2020
32e6e15
Do not use server.ClusterID middleware for v2 receive endpoint
metalmatze Apr 16, 2020
220a33c
Pass transforms into validate middleware from main.go
metalmatze Apr 16, 2020
2154269
Fix unit tests for http middlewares
metalmatze Apr 16, 2020
69ea0e0
Improve GoDoc comment
metalmatze Apr 20, 2020
eba3200
Don't set a timeout on the HTTP client
metalmatze Apr 20, 2020
a072437
Improve error for rate limited cluster
metalmatze Apr 20, 2020
28b3273
Improve and add back all test cases for snappy_test.go
metalmatze Apr 20, 2020
3f24e56
Move clusterIDcontext to the top of the validator file
metalmatze Apr 20, 2020
b6fd58d
Limit the body size before reading it in the validator
metalmatze Apr 20, 2020
b516d66
Fix vlidator_test by passing known now func
metalmatze Apr 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
107 changes: 54 additions & 53 deletions cmd/telemeter-server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,26 @@ import (
"time"

"github.com/coreos/go-oidc"
"github.com/go-chi/chi"
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/oklog/run"
"github.com/prometheus/client_golang/prometheus"
"github.com/spf13/cobra"
"golang.org/x/oauth2"
"golang.org/x/oauth2/clientcredentials"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"

"github.com/openshift/telemeter/pkg/authorize"
"github.com/openshift/telemeter/pkg/authorize/jwt"
"github.com/openshift/telemeter/pkg/authorize/stub"
"github.com/openshift/telemeter/pkg/authorize/tollbooth"
"github.com/openshift/telemeter/pkg/cache"
"github.com/openshift/telemeter/pkg/cache/memcached"
telemeter_http "github.com/openshift/telemeter/pkg/http"
httpserver "github.com/openshift/telemeter/pkg/http/server"
"github.com/openshift/telemeter/pkg/logger"
"github.com/openshift/telemeter/pkg/metricfamily"
"github.com/openshift/telemeter/pkg/receive"
"github.com/openshift/telemeter/pkg/store"
"github.com/openshift/telemeter/pkg/store/forward"
"github.com/openshift/telemeter/pkg/store/ratelimited"
"github.com/openshift/telemeter/pkg/validate"
"github.com/openshift/telemeter/pkg/server"
)

const desc = `
Expand Down Expand Up @@ -81,7 +77,7 @@ func main() {

LimitBytes: 500 * 1024,
TokenExpireSeconds: 24 * 60 * 60,
PartitionKey: "_id",
clusterIDKey: "_id",
Ratelimit: 4*time.Minute + 30*time.Second,
MemcachedExpire: 24 * 60 * 60,
MemcachedInterval: 10,
Expand All @@ -106,7 +102,7 @@ func main() {
cmd.Flags().StringVar(&opt.InternalTLSCertificatePath, "internal-tls-crt", opt.InternalTLSCertificatePath, "Path to a certificate to serve TLS for internal traffic.")

cmd.Flags().StringSliceVar(&opt.LabelFlag, "label", opt.LabelFlag, "Labels to add to each outgoing metric, in key=value form.")
cmd.Flags().StringVar(&opt.PartitionKey, "partition-label", opt.PartitionKey, "The label to separate incoming data on. This label will be required for callers to include.")
cmd.Flags().StringVar(&opt.clusterIDKey, "partition-label", opt.clusterIDKey, "The label to separate incoming data on. This label will be required for callers to include.")

cmd.Flags().StringVar(&opt.SharedKey, "shared-key", opt.SharedKey, "The path to a private key file that will be used to sign authentication requests.")
cmd.Flags().Int64Var(&opt.TokenExpireSeconds, "token-expire-seconds", opt.TokenExpireSeconds, "The expiration of auth tokens in seconds.")
Expand Down Expand Up @@ -171,7 +167,7 @@ type Options struct {
MemcachedExpire int32
MemcachedInterval int32

PartitionKey string
clusterIDKey string
LabelFlag []string
Labels map[string]string
LimitBytes int64
Expand Down Expand Up @@ -280,24 +276,25 @@ func (o *Options) Run() error {
{
internal := http.NewServeMux()

internalPathJSON, _ := json.MarshalIndent(Paths{Paths: []string{"/", "/metrics", "/debug/pprof", "/healthz", "/healthz/ready"}}, "", " ")

internal.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.URL.Path == "/" && req.Method == "GET" {
w.Header().Add("Content-Type", "application/json")
if _, err := w.Write(internalPathJSON); err != nil {
level.Error(o.Logger).Log("msg", "could not write internal paths", "err", err)
}
return
}
w.WriteHeader(http.StatusNotFound)
}))
// TODO: Refactor to not take *http.Mux
telemeter_http.DebugRoutes(internal)
telemeter_http.MetricRoutes(internal)
telemeter_http.HealthRoutes(internal)

r := chi.NewRouter()
r.Mount("/", internal)

r.Get("/", func(w http.ResponseWriter, req *http.Request) {
internalPathJSON, _ := json.MarshalIndent(Paths{Paths: []string{"/", "/metrics", "/debug/pprof", "/healthz", "/healthz/ready"}}, "", " ")

w.Header().Add("Content-Type", "application/json")
if _, err := w.Write(internalPathJSON); err != nil {
level.Error(o.Logger).Log("msg", "could not write internal paths", "err", err)
}
})

s := &http.Server{
Handler: internal,
Handler: r,
}

internalListener, err := net.Listen("tcp", o.ListenInternal)
Expand Down Expand Up @@ -325,7 +322,12 @@ func (o *Options) Run() error {
})
}
{
external := http.NewServeMux()
external := chi.NewRouter()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: above we call the router r and the mux internal and here we are following a different pattern. let's keep it consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the above still needs the internal to begin with. We need to refactor telemeter_http.DebugRoutes etc in another PR and then can have consistency.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no I simply mean we should call the router r in both cases


// TODO: Refactor HealthRoutes to not take *http.Mux
mux := http.NewServeMux()
telemeter_http.HealthRoutes(mux)
external.Mount("/", mux)

// v1 routes
{
Expand Down Expand Up @@ -402,18 +404,12 @@ func (o *Options) Run() error {
clusterAuth = tollbooth.NewAuthorizer(o.Logger, &authorizeClient, authorizeURL)
}

auth := jwt.NewAuthorizeClusterHandler(o.Logger, o.PartitionKey, o.TokenExpireSeconds, signer, o.RequiredLabels, clusterAuth)
validator := validate.New(o.PartitionKey, o.LimitBytes, 24*time.Hour, time.Now)
auth := jwt.NewAuthorizeClusterHandler(o.Logger, o.clusterIDKey, o.TokenExpireSeconds, signer, o.RequiredLabels, clusterAuth)

var store store.Store
u, err := url.Parse(o.ForwardURL)
forwardURL, err := url.Parse(o.ForwardURL)
if err != nil {
return fmt.Errorf("--forward-url must be a valid URL: %v", err)
}
store = forward.New(o.Logger, u, nil)

// Create a rate-limited store with a memory-store as its backend.
store = ratelimited.New(o.Ratelimit, store)

transforms := metricfamily.MultiTransformer{}
transforms.With(whitelister)
Expand All @@ -422,15 +418,25 @@ func (o *Options) Run() error {
}
transforms.With(metricfamily.NewElide(o.ElideLabels...))

server := httpserver.New(o.Logger, store, validator, transforms)

external.Handle("/authorize", telemeter_http.NewInstrumentedHandler("authorize", auth))
external.Handle("/upload",
authorize.NewAuthorizeClientHandler(jwtAuthorizer,
telemeter_http.NewInstrumentedHandler("upload",
http.HandlerFunc(server.Post),
external.Post("/authorize",
server.InstrumentedHandler("authorize",
auth,
).ServeHTTP)

external.Post("/upload",
server.InstrumentedHandler("upload",
authorize.NewAuthorizeClientHandler(jwtAuthorizer,
server.ClusterID(o.clusterIDKey,
server.Ratelimit(o.Ratelimit, time.Now,
server.Snappy(
server.Validate(transforms, 24*time.Hour, o.LimitBytes, time.Now,
server.ForwardHandler(o.Logger, forwardURL),
),
),
),
),
),
),
).ServeHTTP,
)
}

Expand All @@ -447,13 +453,13 @@ func (o *Options) Run() error {
receiver := receive.NewHandler(o.Logger, o.ForwardURL, prometheus.DefaultRegisterer)

external.Handle("/metrics/v1/receive",
telemeter_http.NewInstrumentedHandler("receive",
server.InstrumentedHandler("receive",
authorize.NewHandler(o.Logger, &v2AuthorizeClient, authorizeURL, o.TenantKey,
receive.LimitBodySize(receive.DefaultRequestLimit,
receive.ValidateLabels(
o.Logger,
http.HandlerFunc(receiver.Receive),
o.PartitionKey, // TODO: Enforce the same labels for v1 and v2
o.clusterIDKey, // TODO: Enforce the same labels for v1 and v2
),
),
),
Expand All @@ -463,17 +469,12 @@ func (o *Options) Run() error {

externalPathJSON, _ := json.MarshalIndent(Paths{Paths: []string{"/", "/authorize", "/upload", "/healthz", "/healthz/ready", "/metrics/v1/receive"}}, "", " ")

external.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.URL.Path == "/" && req.Method == "GET" {
w.Header().Add("Content-Type", "application/json")
if _, err := w.Write(externalPathJSON); err != nil {
level.Error(o.Logger).Log("msg", "could not write external paths", "err", err)
}
return
external.Get("/", func(w http.ResponseWriter, req *http.Request) {
w.Header().Add("Content-Type", "application/json")
if _, err := w.Write(externalPathJSON); err != nil {
level.Error(o.Logger).Log("msg", "could not write external paths", "err", err)
}
w.WriteHeader(http.StatusNotFound)
}))
telemeter_http.HealthRoutes(external)
})

s := &http.Server{
Handler: external,
Expand Down