From 63e48160f6720005d7fd54af3bc73e879774fdf3 Mon Sep 17 00:00:00 2001 From: Dalton Hubble Date: Thu, 30 Jun 2016 11:48:55 -0700 Subject: [PATCH] bootcfg/http: Replace capnslog with logrus --- bootcfg/http/cloud.go | 8 ++--- bootcfg/http/cloud_test.go | 13 +++++--- bootcfg/http/generic.go | 4 +-- bootcfg/http/generic_test.go | 13 +++++--- bootcfg/http/grub.go | 6 ++-- bootcfg/http/handlers.go | 26 +++++++-------- bootcfg/http/handlers_test.go | 13 +++++--- bootcfg/http/http.go | 8 +++-- bootcfg/http/http_test.go | 4 ++- bootcfg/http/ignition.go | 12 +++---- bootcfg/http/ignition_test.go | 16 ++++++--- bootcfg/http/ipxe.go | 4 +-- bootcfg/http/ipxe_test.go | 13 +++++--- bootcfg/http/metadata.go | 2 +- bootcfg/http/metadata_test.go | 10 ++++-- bootcfg/http/pixiecore.go | 2 +- bootcfg/http/pixiecore_test.go | 10 ++++-- bootcfg/http/serialize.go | 16 ++++----- bootcfg/http/serialize_test.go | 17 +++++++--- bootcfg/http/server.go | 61 +++++++++++++++++----------------- cmd/bootcfg/main.go | 14 ++++---- 21 files changed, 160 insertions(+), 112 deletions(-) diff --git a/bootcfg/http/cloud.go b/bootcfg/http/cloud.go index 8dcf4e41..dc539291 100644 --- a/bootcfg/http/cloud.go +++ b/bootcfg/http/cloud.go @@ -44,7 +44,7 @@ func (s *Server) cloudHandler(core server.Server) ContextHandler { if group.Metadata != nil { err = json.Unmarshal(group.Metadata, &data) if err != nil { - log.Errorf("error unmarshalling metadata: %v", err) + s.logger.Errorf("error unmarshalling metadata: %v", err) http.NotFound(w, req) return } @@ -55,7 +55,7 @@ func (s *Server) cloudHandler(core server.Server) ContextHandler { // render the template of a cloud config with data var buf bytes.Buffer - err = renderTemplate(&buf, data, contents) + err = s.renderTemplate(&buf, data, contents) if err != nil { http.NotFound(w, req) return @@ -63,14 +63,14 @@ func (s *Server) cloudHandler(core server.Server) ContextHandler { config := buf.String() if !cloudinit.IsCloudConfig(config) && !cloudinit.IsScript(config) { - log.Error("error parsing user-data") + s.logger.Error("error parsing user-data") http.NotFound(w, req) return } if cloudinit.IsCloudConfig(config) { if _, err = cloudinit.NewCloudConfig(config); err != nil { - log.Errorf("error parsing cloud config: %v", err) + s.logger.Errorf("error parsing cloud config: %v", err) http.NotFound(w, req) return } diff --git a/bootcfg/http/cloud_test.go b/bootcfg/http/cloud_test.go index dc1d512d..f1f1dc4f 100644 --- a/bootcfg/http/cloud_test.go +++ b/bootcfg/http/cloud_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -32,7 +33,8 @@ coreos: Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, CloudConfigs: map[string]string{fake.Profile.CloudId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.cloudHandler(c) ctx := withGroup(context.Background(), fake.Group) @@ -46,7 +48,8 @@ coreos: } func TestCloudHandler_MissingCtxProfile(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.cloudHandler(c) w := httptest.NewRecorder() @@ -56,7 +59,8 @@ func TestCloudHandler_MissingCtxProfile(t *testing.T) { } func TestCloudHandler_MissingCloudConfig(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.cloudHandler(c) ctx := withProfile(context.Background(), fake.Profile) @@ -76,7 +80,8 @@ coreos: Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, CloudConfigs: map[string]string{fake.Profile.CloudId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.cloudHandler(c) ctx := withGroup(context.Background(), fake.Group) diff --git a/bootcfg/http/generic.go b/bootcfg/http/generic.go index e7b5d05d..c0294651 100644 --- a/bootcfg/http/generic.go +++ b/bootcfg/http/generic.go @@ -38,7 +38,7 @@ func (s *Server) genericHandler(core server.Server) ContextHandler { if group.Metadata != nil { err = json.Unmarshal(group.Metadata, &data) if err != nil { - log.Errorf("error unmarshalling metadata: %v", err) + s.logger.Errorf("error unmarshalling metadata: %v", err) http.NotFound(w, req) return } @@ -50,7 +50,7 @@ func (s *Server) genericHandler(core server.Server) ContextHandler { // render the template of a generic config with data var buf bytes.Buffer - err = renderTemplate(&buf, data, contents) + err = s.renderTemplate(&buf, data, contents) if err != nil { http.NotFound(w, req) return diff --git a/bootcfg/http/generic_test.go b/bootcfg/http/generic_test.go index 6f568864..a85cb4a6 100644 --- a/bootcfg/http/generic_test.go +++ b/bootcfg/http/generic_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -26,7 +27,8 @@ SERVICE=etcd2 Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, GenericConfigs: map[string]string{fake.Profile.GenericId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.genericHandler(c) ctx := withGroup(context.Background(), fake.Group) @@ -40,7 +42,8 @@ SERVICE=etcd2 } func TestGenericHandler_MissingCtxProfile(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.genericHandler(c) w := httptest.NewRecorder() @@ -50,7 +53,8 @@ func TestGenericHandler_MissingCtxProfile(t *testing.T) { } func TestGenericHandler_MissingCloudConfig(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.genericHandler(c) ctx := withProfile(context.Background(), fake.Profile) @@ -68,7 +72,8 @@ KEY={{.missing_key}} Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, GenericConfigs: map[string]string{fake.Profile.GenericId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.cloudHandler(c) ctx := withGroup(context.Background(), fake.Group) diff --git a/bootcfg/http/grub.go b/bootcfg/http/grub.go index 16e8c30e..27ec9336 100644 --- a/bootcfg/http/grub.go +++ b/bootcfg/http/grub.go @@ -20,7 +20,7 @@ initrdefi {{ range $element := .Initrd }}"{{$element}}" {{end}} // grubHandler returns a handler which renders a GRUB2 config for the // requester. -func grubHandler() ContextHandler { +func (s *Server) grubHandler() ContextHandler { fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) { profile, err := profileFromContext(ctx) if err != nil { @@ -30,12 +30,12 @@ func grubHandler() ContextHandler { var buf bytes.Buffer err = grubTemplate.Execute(&buf, profile.Boot) if err != nil { - log.Errorf("error rendering template: %v", err) + s.logger.Errorf("error rendering template: %v", err) http.NotFound(w, req) return } if _, err := buf.WriteTo(w); err != nil { - log.Errorf("error writing to response: %v", err) + s.logger.Errorf("error writing to response: %v", err) w.WriteHeader(http.StatusInternalServerError) } } diff --git a/bootcfg/http/handlers.go b/bootcfg/http/handlers.go index 1fe869a7..551b4f20 100644 --- a/bootcfg/http/handlers.go +++ b/bootcfg/http/handlers.go @@ -23,15 +23,6 @@ func requireGET(next http.Handler) http.Handler { return http.HandlerFunc(fn) } -// logRequest logs HTTP requests. -func logRequest(next http.Handler) http.Handler { - fn := func(w http.ResponseWriter, req *http.Request) { - log.Debugf("HTTP %s %v", req.Method, req.URL) - next.ServeHTTP(w, req) - } - return http.HandlerFunc(fn) -} - // versionHandler shows the server name and version for root requests. // Otherwise, a 404 is returned. func versionHandler() http.Handler { @@ -45,12 +36,21 @@ func versionHandler() http.Handler { return http.HandlerFunc(fn) } +// logRequest logs HTTP requests. +func (s *Server) logRequest(next http.Handler) http.Handler { + fn := func(w http.ResponseWriter, req *http.Request) { + s.logger.Debugf("HTTP %s %v", req.Method, req.URL) + next.ServeHTTP(w, req) + } + return http.HandlerFunc(fn) +} + // selectGroup selects the Group whose selectors match the query parameters, // adds the Group to the ctx, and calls the next handler. The next handler // should handle a missing Group. -func selectGroup(core server.Server, next ContextHandler) ContextHandler { +func (s *Server) selectGroup(core server.Server, next ContextHandler) ContextHandler { fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) { - attrs := labelsFromRequest(req) + attrs := labelsFromRequest(s.logger, req) // match machine request group, err := core.SelectGroup(ctx, &pb.SelectGroupRequest{Labels: attrs}) if err == nil { @@ -65,9 +65,9 @@ func selectGroup(core server.Server, next ContextHandler) ContextHandler { // selectProfile selects the Profile for the given query parameters, adds the // Profile to the ctx, and calls the next handler. The next handler should // handle a missing profile. -func selectProfile(core server.Server, next ContextHandler) ContextHandler { +func (s *Server) selectProfile(core server.Server, next ContextHandler) ContextHandler { fn := func(ctx context.Context, w http.ResponseWriter, req *http.Request) { - attrs := labelsFromRequest(req) + attrs := labelsFromRequest(s.logger, req) // match machine request profile, err := core.SelectProfile(ctx, &pb.SelectProfileRequest{Labels: attrs}) if err == nil { diff --git a/bootcfg/http/handlers_test.go b/bootcfg/http/handlers_test.go index e800f1a3..52005c6b 100644 --- a/bootcfg/http/handlers_test.go +++ b/bootcfg/http/handlers_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -42,7 +43,9 @@ func TestSelectGroup(t *testing.T) { store := &fake.FixedStore{ Groups: map[string]*storagepb.Group{fake.Group.Id: fake.Group}, } - srv := server.NewServer(&server.Config{Store: store}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) + c := server.NewServer(&server.Config{Store: store}) next := func(ctx context.Context, w http.ResponseWriter, req *http.Request) { group, err := groupFromContext(ctx) assert.Nil(t, err) @@ -53,7 +56,7 @@ func TestSelectGroup(t *testing.T) { // - query params are used to match uuid=a1b2c3d4 to fake.Group // - the fake.Group is added to the context // - next handler is called - h := selectGroup(srv, ContextHandlerFunc(next)) + h := srv.selectGroup(c, ContextHandlerFunc(next)) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil) h.ServeHTTP(context.Background(), w, req) @@ -65,7 +68,9 @@ func TestSelectProfile(t *testing.T) { Groups: map[string]*storagepb.Group{fake.Group.Id: fake.Group}, Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, } - srv := server.NewServer(&server.Config{Store: store}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) + c := server.NewServer(&server.Config{Store: store}) next := func(ctx context.Context, w http.ResponseWriter, req *http.Request) { profile, err := profileFromContext(ctx) assert.Nil(t, err) @@ -76,7 +81,7 @@ func TestSelectProfile(t *testing.T) { // - query params are used to match uuid=a1b2c3d4 to fake.Group's fakeProfile // - the fake.Profile is added to the context // - next handler is called - h := selectProfile(srv, ContextHandlerFunc(next)) + h := srv.selectProfile(c, ContextHandlerFunc(next)) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "?uuid=a1b2c3d4", nil) h.ServeHTTP(context.Background(), w, req) diff --git a/bootcfg/http/http.go b/bootcfg/http/http.go index 2ed0084b..b3849f05 100644 --- a/bootcfg/http/http.go +++ b/bootcfg/http/http.go @@ -5,6 +5,7 @@ import ( "net/http" "strings" + "github.com/Sirupsen/logrus" "golang.org/x/net/context" ) @@ -49,7 +50,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } // labelsFromRequest returns request query parameters. -func labelsFromRequest(req *http.Request) map[string]string { +func labelsFromRequest(logger *logrus.Logger, req *http.Request) map[string]string { values := req.URL.Query() labels := map[string]string{} for key := range values { @@ -58,6 +59,9 @@ func labelsFromRequest(req *http.Request) map[string]string { // set mac if and only if it parses if hw, err := parseMAC(values.Get(key)); err == nil { labels[key] = hw.String() + } else { + // invalid MAC arguments may be common + logger.Debugf("error parsing MAC address: %s", err) } default: // matchers don't use multi-value keys, drop later values @@ -71,8 +75,6 @@ func labelsFromRequest(req *http.Request) map[string]string { func parseMAC(s string) (net.HardwareAddr, error) { macAddr, err := net.ParseMAC(s) if err != nil { - // invalid MAC arguments may be common - log.Debugf("error parsing MAC address: %s", err) return nil, err } return macAddr, err diff --git a/bootcfg/http/http_test.go b/bootcfg/http/http_test.go index 062ecb52..cea43fda 100644 --- a/bootcfg/http/http_test.go +++ b/bootcfg/http/http_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" ) @@ -23,6 +24,7 @@ func TestNewHandler(t *testing.T) { func TestLabelsFromRequest(t *testing.T) { emptyMap := map[string]string{} + logger, _ := logtest.NewNullLogger() cases := []struct { urlString string labels map[string]string @@ -41,6 +43,6 @@ func TestLabelsFromRequest(t *testing.T) { for _, c := range cases { req, err := http.NewRequest("GET", c.urlString, nil) assert.Nil(t, err) - assert.Equal(t, c.labels, labelsFromRequest(req)) + assert.Equal(t, c.labels, labelsFromRequest(logger, req)) } } diff --git a/bootcfg/http/ignition.go b/bootcfg/http/ignition.go index ed037cd0..608197e3 100644 --- a/bootcfg/http/ignition.go +++ b/bootcfg/http/ignition.go @@ -41,9 +41,9 @@ func (s *Server) ignitionHandler(core server.Server) ContextHandler { if isIgnition(profile.IgnitionId) { _, err := ignition.Parse([]byte(contents)) if err != nil { - log.Warningf("warning parsing Ignition JSON: %v", err) + s.logger.Warningf("warning parsing Ignition JSON: %v", err) } - writeJSON(w, []byte(contents)) + s.writeJSON(w, []byte(contents)) return } @@ -54,7 +54,7 @@ func (s *Server) ignitionHandler(core server.Server) ContextHandler { if group.Metadata != nil { err = json.Unmarshal(group.Metadata, &data) if err != nil { - log.Errorf("error unmarshalling metadata: %v", err) + s.logger.Errorf("error unmarshalling metadata: %v", err) http.NotFound(w, req) return } @@ -66,7 +66,7 @@ func (s *Server) ignitionHandler(core server.Server) ContextHandler { // render the template for an Ignition config with data var buf bytes.Buffer - err = renderTemplate(&buf, data, contents) + err = s.renderTemplate(&buf, data, contents) if err != nil { http.NotFound(w, req) return @@ -75,11 +75,11 @@ func (s *Server) ignitionHandler(core server.Server) ContextHandler { // Parse fuze config into an Ignition config config, err := fuze.ParseAsV2_0_0(buf.Bytes()) if err == nil { - renderJSON(w, config) + s.renderJSON(w, config) return } - log.Errorf("error parsing Ignition config: %v", err) + s.logger.Errorf("error parsing Ignition config: %v", err) http.NotFound(w, req) return } diff --git a/bootcfg/http/ignition_test.go b/bootcfg/http/ignition_test.go index ca82f6a4..83122f50 100644 --- a/bootcfg/http/ignition_test.go +++ b/bootcfg/http/ignition_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -28,7 +29,8 @@ func TestIgnitionHandler_V2JSON(t *testing.T) { Profiles: map[string]*storagepb.Profile{fake.Group.Profile: profile}, IgnitionConfigs: map[string]string{"file.ign": content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.ignitionHandler(c) ctx := withGroup(context.Background(), fake.Group) @@ -57,7 +59,8 @@ systemd: Profiles: map[string]*storagepb.Profile{fake.Group.Profile: testProfileIgnitionYAML}, IgnitionConfigs: map[string]string{testProfileIgnitionYAML.IgnitionId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.ignitionHandler(c) ctx := withGroup(context.Background(), fake.Group) @@ -74,7 +77,8 @@ systemd: } func TestIgnitionHandler_MissingCtxProfile(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.ignitionHandler(c) w := httptest.NewRecorder() @@ -84,7 +88,8 @@ func TestIgnitionHandler_MissingCtxProfile(t *testing.T) { } func TestIgnitionHandler_MissingIgnitionConfig(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.ignitionHandler(c) ctx := withProfile(context.Background(), fake.Profile) @@ -106,7 +111,8 @@ systemd: Profiles: map[string]*storagepb.Profile{fake.Group.Profile: fake.Profile}, IgnitionConfigs: map[string]string{fake.Profile.IgnitionId: content}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.ignitionHandler(c) ctx := withGroup(context.Background(), fake.Group) diff --git a/bootcfg/http/ipxe.go b/bootcfg/http/ipxe.go index 2578eb2c..61eb9daf 100644 --- a/bootcfg/http/ipxe.go +++ b/bootcfg/http/ipxe.go @@ -40,12 +40,12 @@ func (s *Server) ipxeHandler() ContextHandler { var buf bytes.Buffer err = ipxeTemplate.Execute(&buf, profile.Boot) if err != nil { - log.Errorf("error rendering template: %v", err) + s.logger.Errorf("error rendering template: %v", err) http.NotFound(w, req) return } if _, err := buf.WriteTo(w); err != nil { - log.Errorf("error writing to response: %v", err) + s.logger.Errorf("error writing to response: %v", err) w.WriteHeader(http.StatusInternalServerError) } } diff --git a/bootcfg/http/ipxe_test.go b/bootcfg/http/ipxe_test.go index 238ed0eb..f7bc00ab 100644 --- a/bootcfg/http/ipxe_test.go +++ b/bootcfg/http/ipxe_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -22,7 +23,8 @@ func TestIPXEInspect(t *testing.T) { } func TestIPXEHandler(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.ipxeHandler() ctx := withProfile(context.Background(), fake.Profile) w := httptest.NewRecorder() @@ -40,7 +42,8 @@ boot } func TestIPXEHandler_MissingCtxProfile(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.ipxeHandler() w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/", nil) @@ -49,7 +52,8 @@ func TestIPXEHandler_MissingCtxProfile(t *testing.T) { } func TestIPXEHandler_RenderTemplateError(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.ipxeHandler() // a Profile with nil NetBoot forces a template.Execute error ctx := withProfile(context.Background(), &storagepb.Profile{Boot: nil}) @@ -60,7 +64,8 @@ func TestIPXEHandler_RenderTemplateError(t *testing.T) { } func TestIPXEHandler_WriteError(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.ipxeHandler() ctx := withProfile(context.Background(), fake.Profile) w := NewUnwriteableResponseWriter() diff --git a/bootcfg/http/metadata.go b/bootcfg/http/metadata.go index 89886cfb..9e5a3cf8 100644 --- a/bootcfg/http/metadata.go +++ b/bootcfg/http/metadata.go @@ -24,7 +24,7 @@ func (s *Server) metadataHandler() ContextHandler { if group.Metadata != nil { err = json.Unmarshal(group.Metadata, &data) if err != nil { - log.Errorf("error unmarshalling metadata: %v", err) + s.logger.Errorf("error unmarshalling metadata: %v", err) http.NotFound(w, req) return } diff --git a/bootcfg/http/metadata_test.go b/bootcfg/http/metadata_test.go index d5e41541..09e0e68b 100644 --- a/bootcfg/http/metadata_test.go +++ b/bootcfg/http/metadata_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -15,7 +16,8 @@ import ( ) func TestMetadataHandler(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.metadataHandler() ctx := withGroup(context.Background(), fake.Group) w := httptest.NewRecorder() @@ -36,7 +38,8 @@ func TestMetadataHandler(t *testing.T) { } func TestMetadataHandler_MetadataEdgeCases(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.metadataHandler() // groups with different metadata cases := []struct { @@ -64,7 +67,8 @@ func TestMetadataHandler_MetadataEdgeCases(t *testing.T) { } func TestMetadataHandler_MissingCtxGroup(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) h := srv.metadataHandler() w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/", nil) diff --git a/bootcfg/http/pixiecore.go b/bootcfg/http/pixiecore.go index 7cf90b9c..7a084c34 100644 --- a/bootcfg/http/pixiecore.go +++ b/bootcfg/http/pixiecore.go @@ -32,7 +32,7 @@ func (s *Server) pixiecoreHandler(core server.Server) ContextHandler { http.NotFound(w, req) return } - renderJSON(w, profile.Boot) + s.renderJSON(w, profile.Boot) } return ContextHandlerFunc(fn) } diff --git a/bootcfg/http/pixiecore_test.go b/bootcfg/http/pixiecore_test.go index 41ff73f5..9ae35aa4 100644 --- a/bootcfg/http/pixiecore_test.go +++ b/bootcfg/http/pixiecore_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -34,7 +35,8 @@ func TestPixiecoreHandler(t *testing.T) { } func TestPixiecoreHandler_InvalidMACAddress(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.pixiecoreHandler(c) w := httptest.NewRecorder() @@ -45,7 +47,8 @@ func TestPixiecoreHandler_InvalidMACAddress(t *testing.T) { } func TestPixiecoreHandler_NoMatchingGroup(t *testing.T) { - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: &fake.EmptyStore{}}) h := srv.pixiecoreHandler(c) w := httptest.NewRecorder() @@ -58,7 +61,8 @@ func TestPixiecoreHandler_NoMatchingProfile(t *testing.T) { store := &fake.FixedStore{ Groups: map[string]*storagepb.Group{fake.Group.Id: fake.Group}, } - srv := NewServer(&Config{}) + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) c := server.NewServer(&server.Config{Store: store}) h := srv.pixiecoreHandler(c) w := httptest.NewRecorder() diff --git a/bootcfg/http/serialize.go b/bootcfg/http/serialize.go index 9fba14d3..c5dfce05 100644 --- a/bootcfg/http/serialize.go +++ b/bootcfg/http/serialize.go @@ -14,38 +14,38 @@ const ( // renderJSON encodes structs to JSON, writes the response to the // ResponseWriter, and logs encoding errors. -func renderJSON(w http.ResponseWriter, v interface{}) { +func (s *Server) renderJSON(w http.ResponseWriter, v interface{}) { js, err := json.Marshal(v) if err != nil { - log.Errorf("error JSON encoding: %v", err) + s.logger.Errorf("error JSON encoding: %v", err) w.WriteHeader(http.StatusInternalServerError) return } - writeJSON(w, js) + s.writeJSON(w, js) } // writeJSON writes the given bytes with a JSON Content-Type. -func writeJSON(w http.ResponseWriter, data []byte) { +func (s *Server) writeJSON(w http.ResponseWriter, data []byte) { w.Header().Set(contentType, jsonContentType) _, err := w.Write(data) if err != nil { - log.Errorf("error writing to response: %v", err) + s.logger.Errorf("error writing to response: %v", err) w.WriteHeader(http.StatusInternalServerError) } } -func renderTemplate(w io.Writer, data interface{}, contents ...string) (err error) { +func (s *Server) renderTemplate(w io.Writer, data interface{}, contents ...string) (err error) { tmpl := template.New("").Option("missingkey=error") for _, content := range contents { tmpl, err = tmpl.Parse(content) if err != nil { - log.Errorf("error parsing template: %v", err) + s.logger.Errorf("error parsing template: %v", err) return err } } err = tmpl.Execute(w, data) if err != nil { - log.Errorf("error rendering template: %v", err) + s.logger.Errorf("error rendering template: %v", err) return err } return nil diff --git a/bootcfg/http/serialize_test.go b/bootcfg/http/serialize_test.go index 0abb3a7d..ed8709cc 100644 --- a/bootcfg/http/serialize_test.go +++ b/bootcfg/http/serialize_test.go @@ -6,39 +6,48 @@ import ( "net/http/httptest" "testing" + logtest "github.com/Sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" ) func TestRenderJSON(t *testing.T) { + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) w := httptest.NewRecorder() data := map[string][]string{ "a": []string{"b", "c"}, } - renderJSON(w, data) + srv.renderJSON(w, data) assert.Equal(t, http.StatusOK, w.Code) assert.Equal(t, jsonContentType, w.HeaderMap.Get(contentType)) assert.Equal(t, `{"a":["b","c"]}`, w.Body.String()) } func TestRenderJSON_EncodingError(t *testing.T) { + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) w := httptest.NewRecorder() // channels cannot be JSON encoded - renderJSON(w, make(chan struct{})) + srv.renderJSON(w, make(chan struct{})) assert.Equal(t, http.StatusInternalServerError, w.Code) assert.Empty(t, w.Body.String()) } func TestRenderJSON_EncodeError(t *testing.T) { + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) w := httptest.NewRecorder() // channels cannot be JSON encoded - renderJSON(w, make(chan struct{})) + srv.renderJSON(w, make(chan struct{})) assert.Equal(t, http.StatusInternalServerError, w.Code) assert.Empty(t, w.Body.String()) } func TestRenderJSON_WriteError(t *testing.T) { + logger, _ := logtest.NewNullLogger() + srv := NewServer(&Config{Logger: logger}) w := NewUnwriteableResponseWriter() - renderJSON(w, map[string]string{"a": "b"}) + srv.renderJSON(w, map[string]string{"a": "b"}) assert.Equal(t, http.StatusInternalServerError, w.Code) assert.Empty(t, w.Body.String()) } diff --git a/bootcfg/http/server.go b/bootcfg/http/server.go index 097c9361..47f2d229 100644 --- a/bootcfg/http/server.go +++ b/bootcfg/http/server.go @@ -3,17 +3,16 @@ package http import ( "net/http" - "github.com/coreos/pkg/capnslog" + "github.com/Sirupsen/logrus" "github.com/coreos/coreos-baremetal/bootcfg/server" "github.com/coreos/coreos-baremetal/bootcfg/sign" ) -var log = capnslog.NewPackageLogger("github.com/coreos/coreos-baremetal/bootcfg", "http") - // Config configures a Server. type Config struct { - Core server.Server + Core server.Server + Logger *logrus.Logger // Path to static assets AssetsPath string // config signers (.sig and .asc) @@ -24,6 +23,7 @@ type Config struct { // Server serves boot and provisioning configs to machines via HTTP. type Server struct { core server.Server + logger *logrus.Logger assetsPath string signer sign.Signer armoredSigner sign.Signer @@ -33,6 +33,7 @@ type Server struct { func NewServer(config *Config) *Server { return &Server{ core: config.Core, + logger: config.Logger, assetsPath: config.AssetsPath, signer: config.Signer, armoredSigner: config.ArmoredSigner, @@ -44,57 +45,57 @@ func (s *Server) HTTPHandler() http.Handler { mux := http.NewServeMux() // bootcfg version - mux.Handle("/", logRequest(versionHandler())) + mux.Handle("/", s.logRequest(versionHandler())) // Boot via GRUB - mux.Handle("/grub", logRequest(NewHandler(selectProfile(s.core, grubHandler())))) + mux.Handle("/grub", s.logRequest(NewHandler(s.selectProfile(s.core, s.grubHandler())))) // Boot via iPXE - mux.Handle("/boot.ipxe", logRequest(ipxeInspect())) - mux.Handle("/boot.ipxe.0", logRequest(ipxeInspect())) - mux.Handle("/ipxe", logRequest(NewHandler(selectProfile(s.core, s.ipxeHandler())))) + mux.Handle("/boot.ipxe", s.logRequest(ipxeInspect())) + mux.Handle("/boot.ipxe.0", s.logRequest(ipxeInspect())) + mux.Handle("/ipxe", s.logRequest(NewHandler(s.selectProfile(s.core, s.ipxeHandler())))) // Boot via Pixiecore - mux.Handle("/pixiecore/v1/boot/", logRequest(NewHandler(s.pixiecoreHandler(s.core)))) + mux.Handle("/pixiecore/v1/boot/", s.logRequest(NewHandler(s.pixiecoreHandler(s.core)))) // Ignition Config - mux.Handle("/ignition", logRequest(NewHandler(selectGroup(s.core, s.ignitionHandler(s.core))))) + mux.Handle("/ignition", s.logRequest(NewHandler(s.selectGroup(s.core, s.ignitionHandler(s.core))))) // Cloud-Config - mux.Handle("/cloud", logRequest(NewHandler(selectGroup(s.core, s.cloudHandler(s.core))))) + mux.Handle("/cloud", s.logRequest(NewHandler(s.selectGroup(s.core, s.cloudHandler(s.core))))) // Generic template - mux.Handle("/generic", logRequest(NewHandler(selectGroup(s.core, s.genericHandler(s.core))))) - // metadata - mux.Handle("/metadata", logRequest(NewHandler(selectGroup(s.core, s.metadataHandler())))) + mux.Handle("/generic", s.logRequest(NewHandler(s.selectGroup(s.core, s.genericHandler(s.core))))) + // Metadata + mux.Handle("/metadata", s.logRequest(NewHandler(s.selectGroup(s.core, s.metadataHandler())))) // Signatures if s.signer != nil { signerChain := func(next http.Handler) http.Handler { - return logRequest(sign.SignatureHandler(s.signer, next)) + return s.logRequest(sign.SignatureHandler(s.signer, next)) } - mux.Handle("/grub.sig", signerChain(NewHandler(selectProfile(s.core, grubHandler())))) + mux.Handle("/grub.sig", signerChain(NewHandler(s.selectProfile(s.core, s.grubHandler())))) mux.Handle("/boot.ipxe.sig", signerChain(ipxeInspect())) mux.Handle("/boot.ipxe.0.sig", signerChain(ipxeInspect())) - mux.Handle("/ipxe.sig", signerChain(NewHandler(selectProfile(s.core, s.ipxeHandler())))) + mux.Handle("/ipxe.sig", signerChain(NewHandler(s.selectProfile(s.core, s.ipxeHandler())))) mux.Handle("/pixiecore/v1/boot.sig/", signerChain(NewHandler(s.pixiecoreHandler(s.core)))) - mux.Handle("/ignition.sig", signerChain(NewHandler(selectGroup(s.core, s.ignitionHandler(s.core))))) - mux.Handle("/cloud.sig", signerChain(NewHandler(selectGroup(s.core, s.cloudHandler(s.core))))) - mux.Handle("/generic.sig", signerChain(NewHandler(selectGroup(s.core, s.genericHandler(s.core))))) - mux.Handle("/metadata.sig", signerChain(NewHandler(selectGroup(s.core, s.metadataHandler())))) + mux.Handle("/ignition.sig", signerChain(NewHandler(s.selectGroup(s.core, s.ignitionHandler(s.core))))) + mux.Handle("/cloud.sig", signerChain(NewHandler(s.selectGroup(s.core, s.cloudHandler(s.core))))) + mux.Handle("/generic.sig", signerChain(NewHandler(s.selectGroup(s.core, s.genericHandler(s.core))))) + mux.Handle("/metadata.sig", signerChain(NewHandler(s.selectGroup(s.core, s.metadataHandler())))) } if s.armoredSigner != nil { signerChain := func(next http.Handler) http.Handler { - return logRequest(sign.SignatureHandler(s.armoredSigner, next)) + return s.logRequest(sign.SignatureHandler(s.armoredSigner, next)) } - mux.Handle("/grub.asc", signerChain(NewHandler(selectProfile(s.core, grubHandler())))) + mux.Handle("/grub.asc", signerChain(NewHandler(s.selectProfile(s.core, s.grubHandler())))) mux.Handle("/boot.ipxe.asc", signerChain(ipxeInspect())) mux.Handle("/boot.ipxe.0.asc", signerChain(ipxeInspect())) - mux.Handle("/ipxe.asc", signerChain(NewHandler(selectProfile(s.core, s.ipxeHandler())))) + mux.Handle("/ipxe.asc", signerChain(NewHandler(s.selectProfile(s.core, s.ipxeHandler())))) mux.Handle("/pixiecore/v1/boot.asc/", signerChain(NewHandler(s.pixiecoreHandler(s.core)))) - mux.Handle("/ignition.asc", signerChain(NewHandler(selectGroup(s.core, s.ignitionHandler(s.core))))) - mux.Handle("/cloud.asc", signerChain(NewHandler(selectGroup(s.core, s.cloudHandler(s.core))))) - mux.Handle("/generic.asc", signerChain(NewHandler(selectGroup(s.core, s.genericHandler(s.core))))) - mux.Handle("/metadata.asc", signerChain(NewHandler(selectGroup(s.core, s.metadataHandler())))) + mux.Handle("/ignition.asc", signerChain(NewHandler(s.selectGroup(s.core, s.ignitionHandler(s.core))))) + mux.Handle("/cloud.asc", signerChain(NewHandler(s.selectGroup(s.core, s.cloudHandler(s.core))))) + mux.Handle("/generic.asc", signerChain(NewHandler(s.selectGroup(s.core, s.genericHandler(s.core))))) + mux.Handle("/metadata.asc", signerChain(NewHandler(s.selectGroup(s.core, s.metadataHandler())))) } // kernel, initrd, and TLS assets if s.assetsPath != "" { - mux.Handle("/assets/", logRequest(http.StripPrefix("/assets/", http.FileServer(http.Dir(s.assetsPath))))) + mux.Handle("/assets/", s.logRequest(http.StripPrefix("/assets/", http.FileServer(http.Dir(s.assetsPath))))) } return mux } diff --git a/cmd/bootcfg/main.go b/cmd/bootcfg/main.go index 3e7604bf..5d595b1b 100644 --- a/cmd/bootcfg/main.go +++ b/cmd/bootcfg/main.go @@ -7,9 +7,8 @@ import ( "net/http" "net/url" "os" - "strings" - "github.com/coreos/pkg/capnslog" + "github.com/Sirupsen/logrus" "github.com/coreos/pkg/flagutil" web "github.com/coreos/coreos-baremetal/bootcfg/http" @@ -22,7 +21,8 @@ import ( ) var ( - log = capnslog.NewPackageLogger("github.com/coreos/coreos-baremetal/cmd/bootcfg", "main") + // Defaults to info logging + log = logrus.New() ) func main() { @@ -44,7 +44,7 @@ func main() { flag.StringVar(&flags.dataPath, "data-path", "/var/lib/bootcfg", "Path to data directory") flag.StringVar(&flags.assetsPath, "assets-path", "/var/lib/bootcfg/assets", "Path to static assets") - // Log levels https://godoc.org/github.com/coreos/pkg/capnslog#LogLevel + // Log levels https://github.com/Sirupsen/logrus/blob/master/logrus.go#L36 flag.StringVar(&flags.logLevel, "log-level", "info", "Set the logging level") // gRPC Server TLS @@ -103,12 +103,11 @@ func main() { } // logging setup - lvl, err := capnslog.ParseLevel(strings.ToUpper(flags.logLevel)) + lvl, err := logrus.ParseLevel(flags.logLevel) if err != nil { log.Fatalf("invalid log-level: %v", err) } - capnslog.SetGlobalLogLevel(lvl) - capnslog.SetFormatter(capnslog.NewPrettyFormatter(os.Stdout, false)) + log.Level = lvl // (optional) signing var signer, armoredSigner sign.Signer @@ -158,6 +157,7 @@ func main() { // HTTP Server config := &web.Config{ Core: server, + Logger: log, AssetsPath: flags.assetsPath, Signer: signer, ArmoredSigner: armoredSigner,