From 6614d8923cee4accb3b35712e7031e2e7f20ab33 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Sat, 16 Nov 2024 08:51:09 -0700 Subject: [PATCH 01/21] Clean up and make logging quieter: Logging all 206 partial content requests from the middleware package creates a lot of messages per ISO request. This stops the middleware logger from logging anything and lets the ISO http func handler decide what to log. In the happy path case we only log that patching happened successfully. Signed-off-by: Jacob Weinstock --- internal/ipxe/http/middleware.go | 7 ++-- internal/iso/iso.go | 57 +++++++++++++++++--------------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/internal/ipxe/http/middleware.go b/internal/ipxe/http/middleware.go index 153cf569..caa6e95e 100644 --- a/internal/ipxe/http/middleware.go +++ b/internal/ipxe/http/middleware.go @@ -27,14 +27,13 @@ func (h *loggingMiddleware) ServeHTTP(w http.ResponseWriter, req *http.Request) if uri == "/metrics" { log = false } - if log { - h.log.V(1).Info("request", "method", method, "uri", uri, "client", client) - } res := &responseWriter{ResponseWriter: w} h.handler.ServeHTTP(res, req) // process the request - if log { + r := res.Header().Get("X-Global-Logging") + + if log && r == "" { h.log.Info("response", "method", method, "uri", uri, "client", client, "duration", time.Since(start), "status", res.statusCode) } } diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 55530553..69cdaa6f 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -45,7 +45,8 @@ type Handler struct { } func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { - h.Logger.V(1).Info("entered the roundtrip func") + log := h.Logger.WithValues("method", req.Method, "url", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr) + log.V(1).Info("starting the patching function") if req.Method != http.MethodHead && req.Method != http.MethodGet { return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotImplemented, http.StatusText(http.StatusNotImplemented)), @@ -56,7 +57,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { } if filepath.Ext(req.URL.Path) != ".iso" { - h.Logger.Info("Extension not supported, only supported type is '.iso'", "path", req.URL.Path) + log.Info("Extension not supported, only supported type is '.iso'", "path", req.URL.Path) return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotFound, http.StatusText(http.StatusNotFound)), StatusCode: http.StatusNotFound, @@ -65,7 +66,6 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - ctx := req.Context() // The incoming request url is expected to have the mac address present. // Fetch the mac and validate if there's a hardware object // associated with the mac. @@ -73,7 +73,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // We serve the iso only if this validation passes. ha, err := getMAC(req.URL.Path) if err != nil { - h.Logger.Info("unable to get the mac address", "error", err) + log.Info("unable to get the mac address", "error", err) return &http.Response{ Status: "400 BAD REQUEST", StatusCode: http.StatusBadRequest, @@ -82,9 +82,9 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - f, err := getFacility(ctx, ha, h.Backend) + f, err := getFacility(req.Context(), ha, h.Backend) if err != nil { - h.Logger.V(1).Info("unable to get facility", "mac", ha, "error", err) + log.Info("unable to get facility", "mac", ha, "error", err) if apierrors.IsNotFound(err) { return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotFound, http.StatusText(http.StatusNotFound)), @@ -101,18 +101,6 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - // The hardware object doesn't contain a specific field for consoles - // right now facility is used instead. - var consoles string - switch { - case f != "" && strings.Contains(f, "console="): - consoles = f - case f != "": - consoles = fmt.Sprintf("%s %s", f, defaultConsoles) - default: - consoles = defaultConsoles - } - // Reverse Proxy modifies the request url to // the same path it received the incoming request. // mac-id is added to the url path to do hardware lookups using the backend reader @@ -122,15 +110,21 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // RoundTripper needs a Transport to execute a HTTP transaction // For our use case the default transport will suffice. resp, err := http.DefaultTransport.RoundTrip(req) - // resp, err := h.RoundTripper.RoundTrip(req) if err != nil { - h.Logger.Info("HTTP request didn't receive a response", "sourceIso", h.SourceISO, "error", err) + log.Info("issue with getting the source ISO", "sourceIso", h.SourceISO, "error", err) return nil, err } + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { + log.Info("the request to get the source ISO returned a status other than ok (200)", "sourceIso", h.SourceISO, "status", resp.Status) + return resp, nil + } + // by setting this header we are telling the logging middleware to not log its default log message. + // we do this because there are a lot of partial content requests. + resp.Header.Set("X-Global-Logging", "false") if req.Method == http.MethodHead { // Fuse client typically make a HEAD request before they start requesting content. - h.Logger.V(1).Info("HTTP HEAD request received, patching only occurs on 206 requests") + log.V(1).Info("HTTP HEAD request received, patching only occurs on 206 requests") return resp, nil } @@ -139,7 +133,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { if resp.StatusCode == http.StatusPartialContent { b, err := io.ReadAll(resp.Body) if err != nil { - h.Logger.Error(err, "reading response bytes", "response", resp.Body) + log.Error(err, "reading response bytes") return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), StatusCode: http.StatusInternalServerError, @@ -148,7 +142,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } if err := resp.Body.Close(); err != nil { - h.Logger.Error(err, "closing response body", "response", resp.Body) + log.Error(err, "closing response body") return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), StatusCode: http.StatusInternalServerError, @@ -157,6 +151,17 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } + // The hardware object doesn't contain a specific field for consoles + // right now facility is used instead. + var consoles string + switch { + case f != "" && strings.Contains(f, "console="): + consoles = fmt.Sprintf("facility=%s", f) + case f != "": + consoles = fmt.Sprintf("facility=%s %s", f, defaultConsoles) + default: + consoles = defaultConsoles + } magicStringPadding := bytes.Repeat([]byte{' '}, len(h.MagicString)) // TODO: revisit later to handle the magic string potentially being spread across two chunks. @@ -164,18 +169,18 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // magic string spread across multiple response bodies in the future. i := bytes.Index(b, []byte(h.MagicString)) if i != -1 { - h.Logger.Info("Magic string found, patching the iso at runtime") + log.Info("magic string found, patching the ISO") dup := make([]byte, len(b)) copy(dup, b) copy(dup[i:], magicStringPadding) - copy(dup[i:], []byte(h.constructPatch(fmt.Sprintf("facility=%s", consoles), ha.String()))) + copy(dup[i:], []byte(h.constructPatch(consoles, ha.String()))) b = dup } resp.Body = io.NopCloser(bytes.NewReader(b)) } - h.Logger.Info("roundtrip complete") + log.V(1).Info("roundtrip complete") return resp, nil } From bf7043d44bf1cecd766d81c7b0613edb9ca04eb2 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Mon, 18 Nov 2024 11:49:02 -0700 Subject: [PATCH 02/21] Use an existing logger for sampling: This library allows for sampling log messages that match some certain criteria. I have not implemented this in Smee, but this could allow for sampling each unique mac address in the url path. This would be as opposed to having all traffic be sampled together. This needs tested to see if it's worth it. If it's not there are other ways to do the sampling in the same way without needing to import a library. Signed-off-by: Jacob Weinstock --- cmd/smee/main.go | 14 ++++ go.mod | 6 ++ go.sum | 10 +++ internal/ipxe/http/middleware.go | 3 + internal/iso/iso.go | 130 +++++++++++++++++-------------- internal/iso/iso_test.go | 19 ++++- 6 files changed, 122 insertions(+), 60 deletions(-) diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 6c839130..01c45828 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -20,6 +20,8 @@ import ( "github.com/go-logr/logr" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/server4" + slogmulti "github.com/samber/slog-multi" + slogsampling "github.com/samber/slog-sampling" "github.com/tinkerbell/ipxedust" "github.com/tinkerbell/ipxedust/ihttp" "github.com/tinkerbell/smee/internal/dhcp/handler" @@ -254,6 +256,17 @@ func main() { if err != nil { panic(fmt.Errorf("failed to create backend: %w", err)) } + // Will print 10% of entries. + option := slogsampling.UniformSamplingOption{ + // The sample rate for sampling traces in the range [0.0, 1.0]. + Rate: 0.002, + } + + logger := slog.New( + slogmulti. + Pipe(option.NewMiddleware()). + Handler(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})), + ) ih := iso.Handler{ Logger: log, Backend: br, @@ -268,6 +281,7 @@ func main() { } return cfg.iso.magicString }(), + SampleLogger: logger, } isoHandler, err := ih.Reverse() if err != nil { diff --git a/go.mod b/go.mod index d832edb6..8e897624 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,8 @@ require ( github.com/packethost/xff v0.0.0-20190305172552-d3e9190c41b3 github.com/peterbourgon/ff/v3 v3.4.0 github.com/prometheus/client_golang v1.20.5 + github.com/samber/slog-multi v1.2.4 + github.com/samber/slog-sampling v1.5.2 github.com/tinkerbell/ipxedust v0.0.0-20241108174245-aa0c0298057d github.com/tinkerbell/tink v0.12.1 github.com/vishvananda/netlink v1.3.0 @@ -35,8 +37,10 @@ require ( require ( dario.cat/mergo v1.0.1 // indirect github.com/beorn7/perks v1.0.1 // indirect + github.com/bluele/gcache v0.0.2 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/cornelk/hashmap v1.0.8 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect @@ -78,6 +82,8 @@ require ( github.com/prometheus/common v0.55.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/rs/zerolog v1.33.0 // indirect + github.com/samber/lo v1.47.0 // indirect + github.com/samber/slog-common v0.17.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect github.com/vishvananda/netns v0.0.4 // indirect diff --git a/go.sum b/go.sum index 14f282aa..79c67fa9 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyY github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= +github.com/cornelk/hashmap v1.0.8 h1:nv0AWgw02n+iDcawr5It4CjQIAcdMMKRrs10HOJYlrc= +github.com/cornelk/hashmap v1.0.8/go.mod h1:RfZb7JO3RviW/rT6emczVuC/oxpdz4UsSB2LJSclR1k= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -150,6 +152,14 @@ github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWN github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= +github.com/samber/lo v1.47.0 h1:z7RynLwP5nbyRscyvcD043DWYoOcYRv3mV8lBeqOCLc= +github.com/samber/lo v1.47.0/go.mod h1:RmDH9Ct32Qy3gduHQuKJ3gW1fMHAnE/fAzQuf6He5cU= +github.com/samber/slog-common v0.17.1 h1:jTqqLBgoJshpoxlPSGiypyOanjH6tY+i9bwyYmIbjhI= +github.com/samber/slog-common v0.17.1/go.mod h1:mZSJhinB4aqHziR0SKPqpVZjJ0JO35JfH+dDIWqaCBk= +github.com/samber/slog-multi v1.2.4 h1:k9x3JAWKJFPKffx+oXZ8TasaNuorIW4tG+TXxkt6Ry4= +github.com/samber/slog-multi v1.2.4/go.mod h1:ACuZ5B6heK57TfMVkVknN2UZHoFfjCwRxR0Q2OXKHlo= +github.com/samber/slog-sampling v1.5.2 h1:HaQmRGmLkVsXlVHEgIzgvxc6JYkkppL/7GN1B1g5LQM= +github.com/samber/slog-sampling v1.5.2/go.mod h1:n2PVbLAFRx8Im0KFt9srRrO/8wi9FAiTNBV7FmhbAHY= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/ipxe/http/middleware.go b/internal/ipxe/http/middleware.go index caa6e95e..e079302c 100644 --- a/internal/ipxe/http/middleware.go +++ b/internal/ipxe/http/middleware.go @@ -31,6 +31,9 @@ func (h *loggingMiddleware) ServeHTTP(w http.ResponseWriter, req *http.Request) res := &responseWriter{ResponseWriter: w} h.handler.ServeHTTP(res, req) // process the request + // The "X-Global-Logging" header allows all registered HTTP handlers to disable this global logging + // by setting the header to any non empty string. This is useful for handlers that handle partial content of + // larger file. The ISO handler, for example. r := res.Header().Get("X-Global-Logging") if log && r == "" { diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 69cdaa6f..8b286a5c 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "log/slog" "net" "net/http" "net/http/httputil" @@ -13,6 +14,7 @@ import ( "path" "path/filepath" "strings" + "sync/atomic" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -21,7 +23,7 @@ import ( ) const ( - defaultConsoles = "console=ttyS1 console=ttyS1 console=ttyS0 console=ttyAMA0 console=ttyS1 console=tty0" + defaultConsoles = "console=ttyS0 console=ttyAMA0 console=tty1 console=tty0, console=ttyS1,115200" ) type Handler struct { @@ -41,12 +43,17 @@ type Handler struct { // in the source iso before patching. The field can be set // during build time by setting this field. // Ref: https://github.com/tinkerbell/hook/blob/main/linuxkit-templates/hook.template.yaml - MagicString string + MagicString string + SampleLogger *slog.Logger } +var total atomic.Int64 + func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { - log := h.Logger.WithValues("method", req.Method, "url", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr) - log.V(1).Info("starting the patching function") + log := h.Logger.WithValues("method", req.Method, "urlPath", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr, "total", total.Load()) + splog := h.SampleLogger.With("method", req.Method, "urlPath", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr, "total", total.Load()) + total.Add(1) + log.V(1).Info("starting the ISO patching HTTP handler") if req.Method != http.MethodHead && req.Method != http.MethodGet { return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotImplemented, http.StatusText(http.StatusNotImplemented)), @@ -57,7 +64,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { } if filepath.Ext(req.URL.Path) != ".iso" { - log.Info("Extension not supported, only supported type is '.iso'", "path", req.URL.Path) + log.Info("extension not supported, only supported extension is '.iso'") return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotFound, http.StatusText(http.StatusNotFound)), StatusCode: http.StatusNotFound, @@ -73,9 +80,9 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // We serve the iso only if this validation passes. ha, err := getMAC(req.URL.Path) if err != nil { - log.Info("unable to get the mac address", "error", err) + log.Info("unable to parse mac address in the URL path", "error", err) return &http.Response{ - Status: "400 BAD REQUEST", + Status: fmt.Sprintf("%d %s", http.StatusBadRequest, http.StatusText(http.StatusBadRequest)), StatusCode: http.StatusBadRequest, Body: http.NoBody, Request: req, @@ -84,7 +91,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { f, err := getFacility(req.Context(), ha, h.Backend) if err != nil { - log.Info("unable to get facility", "mac", ha, "error", err) + log.Error(err, "unable to get the hardware object", "mac", ha) if apierrors.IsNotFound(err) { return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusNotFound, http.StatusText(http.StatusNotFound)), @@ -111,7 +118,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // For our use case the default transport will suffice. resp, err := http.DefaultTransport.RoundTrip(req) if err != nil { - log.Info("issue with getting the source ISO", "sourceIso", h.SourceISO, "error", err) + log.Info("issue getting the source ISO", "sourceIso", h.SourceISO, "error", err) return nil, err } if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { @@ -123,63 +130,68 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { resp.Header.Set("X-Global-Logging", "false") if req.Method == http.MethodHead { - // Fuse client typically make a HEAD request before they start requesting content. - log.V(1).Info("HTTP HEAD request received, patching only occurs on 206 requests") + // Fuse clients typically make a HEAD request before they start requesting content. + log.Info("HTTP HEAD method received") return resp, nil } - // roundtripper should only return error when no response from the server - // for any other case just log the error and return 404 response - if resp.StatusCode == http.StatusPartialContent { - b, err := io.ReadAll(resp.Body) - if err != nil { - log.Error(err, "reading response bytes") - return &http.Response{ - Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), - StatusCode: http.StatusInternalServerError, - Body: http.NoBody, - Request: req, - }, nil - } - if err := resp.Body.Close(); err != nil { - log.Error(err, "closing response body") - return &http.Response{ - Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), - StatusCode: http.StatusInternalServerError, - Body: http.NoBody, - Request: req, - }, nil - } + if resp.StatusCode == http.StatusOK { + log.Info("HTTP GET method received with a 200 status code") + return resp, nil + } - // The hardware object doesn't contain a specific field for consoles - // right now facility is used instead. - var consoles string - switch { - case f != "" && strings.Contains(f, "console="): - consoles = fmt.Sprintf("facility=%s", f) - case f != "": - consoles = fmt.Sprintf("facility=%s %s", f, defaultConsoles) - default: - consoles = defaultConsoles - } - magicStringPadding := bytes.Repeat([]byte{' '}, len(h.MagicString)) - - // TODO: revisit later to handle the magic string potentially being spread across two chunks. - // In current implementation we will never patch the above case. Add logic to patch the case of - // magic string spread across multiple response bodies in the future. - i := bytes.Index(b, []byte(h.MagicString)) - if i != -1 { - log.Info("magic string found, patching the ISO") - dup := make([]byte, len(b)) - copy(dup, b) - copy(dup[i:], magicStringPadding) - copy(dup[i:], []byte(h.constructPatch(consoles, ha.String()))) - b = dup - } + splog.Info("HTTP GET method received with a 206 status code") - resp.Body = io.NopCloser(bytes.NewReader(b)) + // this roundtripper func should only return error when there is no response from the server. + // for any other case we log the error and return a 500 response + b, err := io.ReadAll(resp.Body) + if err != nil { + log.Error(err, "issue reading response bytes") + return &http.Response{ + Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), + StatusCode: http.StatusInternalServerError, + Body: http.NoBody, + Request: req, + }, nil + } + if err := resp.Body.Close(); err != nil { + log.Error(err, "issue closing response body") + return &http.Response{ + Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), + StatusCode: http.StatusInternalServerError, + Body: http.NoBody, + Request: req, + }, nil } + // The hardware object doesn't contain a dedicated field for consoles right now and + // historically the facility is used as a way to define consoles on a per Hardware basis. + var consoles string + switch { + case f != "" && strings.Contains(f, "console="): + consoles = fmt.Sprintf("facility=%s", f) + case f != "": + consoles = fmt.Sprintf("facility=%s %s", f, defaultConsoles) + default: + consoles = defaultConsoles + } + magicStringPadding := bytes.Repeat([]byte{' '}, len(h.MagicString)) + + // TODO: revisit later to handle the magic string potentially being spread across two chunks. + // In current implementation we will never patch the above case. Add logic to patch the case of + // magic string spread across multiple response bodies in the future. + i := bytes.Index(b, []byte(h.MagicString)) + if i != -1 { + log.Info("magic string found, patching the content", "contentRange", resp.Header.Get("Content-Range")) + dup := make([]byte, len(b)) + copy(dup, b) + copy(dup[i:], magicStringPadding) + copy(dup[i:], []byte(h.constructPatch(consoles, ha.String()))) + b = dup + } + + resp.Body = io.NopCloser(bytes.NewReader(b)) + log.V(1).Info("roundtrip complete") return resp, nil } diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 63f878d4..f2325f37 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -1,9 +1,14 @@ package iso import ( + "log/slog" "net/http" "net/url" + "os" "testing" + + slogmulti "github.com/samber/slog-multi" + slogsampling "github.com/samber/slog-sampling" ) func TestReqPathInvalid(t *testing.T) { @@ -18,8 +23,20 @@ func TestReqPathInvalid(t *testing.T) { for name, tt := range tests { u, _ := url.Parse(tt.isoURL) t.Run(name, func(t *testing.T) { + // Will print 10% of entries. + option := slogsampling.UniformSamplingOption{ + // The sample rate for sampling traces in the range [0.0, 1.0]. + Rate: 0.002, + } + + logger := slog.New( + slogmulti. + Pipe(option.NewMiddleware()). + Handler(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})), + ) h := &Handler{ - parsedURL: u, + parsedURL: u, + SampleLogger: logger, } req := http.Request{ Method: http.MethodGet, From fb92971c68708943b7ae2ff1216c67653f736839 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Mon, 18 Nov 2024 16:22:50 -0700 Subject: [PATCH 03/21] Update Tilt: Remove all old manifests and use Tilt to deploy the stack via Helm and rebuild Smee and deploy a new image on changes. Signed-off-by: Jacob Weinstock --- Tiltfile | 32 +++++++-- internal/iso/iso.go | 2 +- manifests/kind/config.yaml | 21 ------ manifests/kustomize/base/deployment.yaml | 65 ------------------- manifests/kustomize/base/kustomization.yaml | 13 ---- manifests/kustomize/base/namespace.yaml | 4 -- manifests/kustomize/base/rbac.yaml | 46 ------------- manifests/kustomize/overlays/dev/ingress.yaml | 17 ----- .../kustomize/overlays/dev/kustomization.yaml | 10 --- manifests/kustomize/overlays/dev/service.yaml | 26 -------- .../overlays/k3d/deployment_patch.yaml | 3 - .../kustomize/overlays/k3d/kustomization.yaml | 13 ---- .../overlays/kind/deployment_patch.yaml | 16 ----- .../overlays/kind/kustomization.yaml | 13 ---- 14 files changed, 28 insertions(+), 253 deletions(-) delete mode 100644 manifests/kind/config.yaml delete mode 100644 manifests/kustomize/base/deployment.yaml delete mode 100644 manifests/kustomize/base/kustomization.yaml delete mode 100644 manifests/kustomize/base/namespace.yaml delete mode 100644 manifests/kustomize/base/rbac.yaml delete mode 100644 manifests/kustomize/overlays/dev/ingress.yaml delete mode 100644 manifests/kustomize/overlays/dev/kustomization.yaml delete mode 100644 manifests/kustomize/overlays/dev/service.yaml delete mode 100644 manifests/kustomize/overlays/k3d/deployment_patch.yaml delete mode 100644 manifests/kustomize/overlays/k3d/kustomization.yaml delete mode 100644 manifests/kustomize/overlays/kind/deployment_patch.yaml delete mode 100644 manifests/kustomize/overlays/kind/kustomization.yaml diff --git a/Tiltfile b/Tiltfile index c4b3a7d8..7f2d2c66 100644 --- a/Tiltfile +++ b/Tiltfile @@ -1,11 +1,33 @@ -local_resource( - 'compile smee', - 'make cmd/smee/smee-linux-amd64' +local_resource('compile smee', + cmd='make cmd/smee/smee-linux-amd64', + deps=["go.mod", "go.sum", "internal", "Dockerfile", "cmd/smee/main.go", "cmd/smee/flag.go", "cmd/smee/backend.go"] ) docker_build( 'quay.io/tinkerbell/smee', '.', dockerfile='Dockerfile', - only=['.'] ) -k8s_yaml(kustomize('./manifests/kustomize/overlays/kind')) +#k8s_yaml(kustomize('./manifests/kustomize/overlays/k3d')) +default_registry('ttl.sh/meohmy-dghentld') + +trusted_proxies = os.getenv('trusted_proxies', '') +lb_ip = os.getenv('LB_IP', '192.168.2.114') +stack_version = os.getenv('STACK_CHART_VERSION', '0.5.0') +layer2_interface = os.getenv('LAYER2_INTERFACE', 'eth1') + +load('ext://helm_resource', 'helm_resource') +helm_resource('stack', + chart='oci://ghcr.io/tinkerbell/charts/stack', + namespace='tink', + image_deps=['quay.io/tinkerbell/smee'], + image_keys=[('smee.image')], + flags=[ + '--create-namespace', + '--version=%s' % stack_version, + '--set=global.trustedProxies={%s}' % trusted_proxies, + '--set=global.publicIP=%s' % lb_ip, + '--set=stack.kubevip.interface=%s' % layer2_interface, + '--set=stack.relay.sourceInterface=%s' % layer2_interface, + ], + release_name='tink-stack' +) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 8b286a5c..b695165e 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -122,7 +122,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return nil, err } if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { - log.Info("the request to get the source ISO returned a status other than ok (200)", "sourceIso", h.SourceISO, "status", resp.Status) + log.Info("the request to get the source ISO returned a status other than ok (200) or partial content (206)", "sourceIso", h.SourceISO, "status", resp.Status) return resp, nil } // by setting this header we are telling the logging middleware to not log its default log message. diff --git a/manifests/kind/config.yaml b/manifests/kind/config.yaml deleted file mode 100644 index d1811f0a..00000000 --- a/manifests/kind/config.yaml +++ /dev/null @@ -1,21 +0,0 @@ -kind: Cluster -apiVersion: kind.x-k8s.io/v1alpha4 -nodes: - - role: control-plane - extraPortMappings: - - containerPort: 67 - hostPort: 6767 - listenAddress: "0.0.0.0" - protocol: udp - - containerPort: 69 - hostPort: 69 - listenAddress: "0.0.0.0" - protocol: udp - - containerPort: 514 - hostPort: 514 - listenAddress: "0.0.0.0" - protocol: udp - - containerPort: 80 - hostPort: 80 - listenAddress: "0.0.0.0" - protocol: tcp diff --git a/manifests/kustomize/base/deployment.yaml b/manifests/kustomize/base/deployment.yaml deleted file mode 100644 index 92e0e762..00000000 --- a/manifests/kustomize/base/deployment.yaml +++ /dev/null @@ -1,65 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - app: tinkerbell-smee - name: smee-deployment - namespace: tink-system -spec: - selector: - matchLabels: - app: tinkerbell-smee - strategy: - type: Recreate - template: - metadata: - labels: - app: tinkerbell-smee - spec: - containers: - - args: ["-log-level", "debug", "-dhcp-addr", "0.0.0.0:67"] - env: - - name: SMEE_EXTRA_KERNEL_ARGS - value: "tink_worker_image=quay.io/tinkerbell/tink-worker:latest" - - name: DATA_MODEL_VERSION - value: "kubernetes" - - name: FACILITY_CODE - value: "lab1" - - name: HTTP_BIND - value: ":80" - - name: MIRROR_BASE_URL - value: http://192.168.2.59:8080 - - name: PUBLIC_IP - value: 192.168.2.59 - - name: PUBLIC_SYSLOG_FQDN - value: 192.168.2.59 - - name: SYSLOG_BIND - value: 0.0.0.0:514 - - name: TINKERBELL_GRPC_AUTHORITY - value: 192.168.2.59:42113 - - name: TINKERBELL_TLS - value: "false" - image: smee:latest - imagePullPolicy: IfNotPresent - name: tinkerbell-smee - ports: - - name: dhcp - containerPort: 67 - protocol: UDP - - name: tftp - containerPort: 69 - protocol: UDP - - name: syslog - containerPort: 514 - protocol: UDP - - name: http - containerPort: 80 - protocol: TCP - resources: - limits: - cpu: 500m - memory: 128Mi - requests: - cpu: 10m - memory: 64Mi - serviceAccountName: tinkerbell-smee diff --git a/manifests/kustomize/base/kustomization.yaml b/manifests/kustomize/base/kustomization.yaml deleted file mode 100644 index 1c61210a..00000000 --- a/manifests/kustomize/base/kustomization.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -namespace: tink-system - -images: - - name: quay.io/tinkerbell/smee - newTag: latest -resources: - - github.com/tinkerbell/tink/config/crd?ref=main - - namespace.yaml - - rbac.yaml - - deployment.yaml diff --git a/manifests/kustomize/base/namespace.yaml b/manifests/kustomize/base/namespace.yaml deleted file mode 100644 index 34cfce72..00000000 --- a/manifests/kustomize/base/namespace.yaml +++ /dev/null @@ -1,4 +0,0 @@ -apiVersion: v1 -kind: Namespace -metadata: - name: tink-system diff --git a/manifests/kustomize/base/rbac.yaml b/manifests/kustomize/base/rbac.yaml deleted file mode 100644 index bf38f6be..00000000 --- a/manifests/kustomize/base/rbac.yaml +++ /dev/null @@ -1,46 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - name: tinkerbell-smee - namespace: tink-system ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: tinkerbell-smee-role - namespace: tink-system -rules: - - apiGroups: - - tinkerbell.org - resources: - - hardware - - hardware/status - verbs: - - get - - list - - watch - - apiGroups: - - tinkerbell.org - resources: - - workflows - - workflows/status - verbs: - - get - - list - - watch ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: tinkerbell-smee-role - namespace: tink-system -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: tinkerbell-smee-role -subjects: - - kind: ServiceAccount - name: tinkerbell-smee - namespace: tink-system ---- - diff --git a/manifests/kustomize/overlays/dev/ingress.yaml b/manifests/kustomize/overlays/dev/ingress.yaml deleted file mode 100644 index e20bf7c4..00000000 --- a/manifests/kustomize/overlays/dev/ingress.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: networking.k8s.io/v1 -kind: Ingress -metadata: - name: smee-ingress - namespace: tinkerbell -spec: - ingressClassName: nginx - rules: - - http: - paths: - - pathType: Prefix - path: "/" - backend: - service: - name: smee-svc - port: - number: 80 diff --git a/manifests/kustomize/overlays/dev/kustomization.yaml b/manifests/kustomize/overlays/dev/kustomization.yaml deleted file mode 100644 index 730c2ace..00000000 --- a/manifests/kustomize/overlays/dev/kustomization.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -namespace: tink-system - -resources: - - ./../../base - - service.yaml - - github.com/kubernetes/ingress-nginx/deploy/static/provider/cloud?ref=controller-v1.2.0 - - ingress.yaml diff --git a/manifests/kustomize/overlays/dev/service.yaml b/manifests/kustomize/overlays/dev/service.yaml deleted file mode 100644 index 199a1f04..00000000 --- a/manifests/kustomize/overlays/dev/service.yaml +++ /dev/null @@ -1,26 +0,0 @@ -apiVersion: v1 -kind: Service -metadata: - labels: - app: tinkerbell-smee - name: smee-svc - namespace: tinkerbell -spec: - ports: - - name: dhcp - port: 67 - protocol: UDP - targetPort: dhcp - - name: tftp - port: 69 - protocol: UDP - targetPort: tftp - - name: syslog - port: 514 - protocol: UDP - targetPort: syslog - - name: http - port: 80 - targetPort: http - selector: - app: tinkerbell-smee diff --git a/manifests/kustomize/overlays/k3d/deployment_patch.yaml b/manifests/kustomize/overlays/k3d/deployment_patch.yaml deleted file mode 100644 index 17dcb9fa..00000000 --- a/manifests/kustomize/overlays/k3d/deployment_patch.yaml +++ /dev/null @@ -1,3 +0,0 @@ -- op: add - path: /spec/template/spec/hostNetwork - value: true diff --git a/manifests/kustomize/overlays/k3d/kustomization.yaml b/manifests/kustomize/overlays/k3d/kustomization.yaml deleted file mode 100644 index 56f2d3ee..00000000 --- a/manifests/kustomize/overlays/k3d/kustomization.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -namespace: tink-system - -resources: - - ./../../base - -patches: - - path: deployment_patch.yaml - target: - kind: Deployment - labelSelector: "app=tinkerbell-smee" diff --git a/manifests/kustomize/overlays/kind/deployment_patch.yaml b/manifests/kustomize/overlays/kind/deployment_patch.yaml deleted file mode 100644 index b7312e08..00000000 --- a/manifests/kustomize/overlays/kind/deployment_patch.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# name: dhcp -- op: add - path: /spec/template/spec/containers/0/ports/0/hostPort - value: 67 -# name: tftp -- op: add - path: /spec/template/spec/containers/0/ports/1/hostPort - value: 69 -# name: syslog -- op: add - path: /spec/template/spec/containers/0/ports/2/hostPort - value: 514 -# name: http -- op: add - path: /spec/template/spec/containers/0/ports/3/hostPort - value: 80 diff --git a/manifests/kustomize/overlays/kind/kustomization.yaml b/manifests/kustomize/overlays/kind/kustomization.yaml deleted file mode 100644 index 56f2d3ee..00000000 --- a/manifests/kustomize/overlays/kind/kustomization.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization - -namespace: tink-system - -resources: - - ./../../base - -patches: - - path: deployment_patch.yaml - target: - kind: Deployment - labelSelector: "app=tinkerbell-smee" From 6e76e0f57799caa67295df73cc01165ad977d633 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Mon, 18 Nov 2024 16:39:15 -0700 Subject: [PATCH 04/21] Get go.sum updated: There was a merge conflict and I removed needed go.sum lines. This adds them back. Signed-off-by: Jacob Weinstock --- go.sum | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.sum b/go.sum index 79c67fa9..a6193065 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ dario.cat/mergo v1.0.1 h1:Ra4+bf83h2ztPIQYNP99R6m+Y7KfnARDfID+a+vLl4s= dario.cat/mergo v1.0.1/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw= +github.com/bluele/gcache v0.0.2/go.mod h1:m15KV+ECjptwSPxKhOhQoAFQVtUFjTVkc3H8o0t/fp0= github.com/ccoveille/go-safecast v1.2.0 h1:H4X7aosepsU1Mfk+098CTdKpsDH0cfYJ2RmwXFjgvfc= github.com/ccoveille/go-safecast v1.2.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= From 85d33bcfc16b231f52331d1641a41d122d2e3b97 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 14:41:04 -0700 Subject: [PATCH 05/21] Use a basic percentage base sampling: The sampling import isn't needed for now. The most simple case of using a random number below a percentage will suffice for now. Signed-off-by: Jacob Weinstock --- cmd/smee/main.go | 15 --------------- go.mod | 6 ------ go.sum | 12 ------------ internal/iso/iso.go | 30 ++++++++++++++++++++---------- internal/iso/iso_test.go | 19 +------------------ 5 files changed, 21 insertions(+), 61 deletions(-) diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 01c45828..814c2890 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -5,7 +5,6 @@ import ( "errors" "flag" "fmt" - "log/slog" "net" "net/netip" "net/url" @@ -20,8 +19,6 @@ import ( "github.com/go-logr/logr" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/insomniacslk/dhcp/dhcpv4/server4" - slogmulti "github.com/samber/slog-multi" - slogsampling "github.com/samber/slog-sampling" "github.com/tinkerbell/ipxedust" "github.com/tinkerbell/ipxedust/ihttp" "github.com/tinkerbell/smee/internal/dhcp/handler" @@ -256,17 +253,6 @@ func main() { if err != nil { panic(fmt.Errorf("failed to create backend: %w", err)) } - // Will print 10% of entries. - option := slogsampling.UniformSamplingOption{ - // The sample rate for sampling traces in the range [0.0, 1.0]. - Rate: 0.002, - } - - logger := slog.New( - slogmulti. - Pipe(option.NewMiddleware()). - Handler(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})), - ) ih := iso.Handler{ Logger: log, Backend: br, @@ -281,7 +267,6 @@ func main() { } return cfg.iso.magicString }(), - SampleLogger: logger, } isoHandler, err := ih.Reverse() if err != nil { diff --git a/go.mod b/go.mod index 8e897624..d832edb6 100644 --- a/go.mod +++ b/go.mod @@ -15,8 +15,6 @@ require ( github.com/packethost/xff v0.0.0-20190305172552-d3e9190c41b3 github.com/peterbourgon/ff/v3 v3.4.0 github.com/prometheus/client_golang v1.20.5 - github.com/samber/slog-multi v1.2.4 - github.com/samber/slog-sampling v1.5.2 github.com/tinkerbell/ipxedust v0.0.0-20241108174245-aa0c0298057d github.com/tinkerbell/tink v0.12.1 github.com/vishvananda/netlink v1.3.0 @@ -37,10 +35,8 @@ require ( require ( dario.cat/mergo v1.0.1 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/bluele/gcache v0.0.2 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/cornelk/hashmap v1.0.8 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect @@ -82,8 +78,6 @@ require ( github.com/prometheus/common v0.55.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/rs/zerolog v1.33.0 // indirect - github.com/samber/lo v1.47.0 // indirect - github.com/samber/slog-common v0.17.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect github.com/vishvananda/netns v0.0.4 // indirect diff --git a/go.sum b/go.sum index a6193065..14f282aa 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,6 @@ dario.cat/mergo v1.0.1 h1:Ra4+bf83h2ztPIQYNP99R6m+Y7KfnARDfID+a+vLl4s= dario.cat/mergo v1.0.1/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= -github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw= -github.com/bluele/gcache v0.0.2/go.mod h1:m15KV+ECjptwSPxKhOhQoAFQVtUFjTVkc3H8o0t/fp0= github.com/ccoveille/go-safecast v1.2.0 h1:H4X7aosepsU1Mfk+098CTdKpsDH0cfYJ2RmwXFjgvfc= github.com/ccoveille/go-safecast v1.2.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= @@ -11,8 +9,6 @@ github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyY github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= -github.com/cornelk/hashmap v1.0.8 h1:nv0AWgw02n+iDcawr5It4CjQIAcdMMKRrs10HOJYlrc= -github.com/cornelk/hashmap v1.0.8/go.mod h1:RfZb7JO3RviW/rT6emczVuC/oxpdz4UsSB2LJSclR1k= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -154,14 +150,6 @@ github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWN github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= -github.com/samber/lo v1.47.0 h1:z7RynLwP5nbyRscyvcD043DWYoOcYRv3mV8lBeqOCLc= -github.com/samber/lo v1.47.0/go.mod h1:RmDH9Ct32Qy3gduHQuKJ3gW1fMHAnE/fAzQuf6He5cU= -github.com/samber/slog-common v0.17.1 h1:jTqqLBgoJshpoxlPSGiypyOanjH6tY+i9bwyYmIbjhI= -github.com/samber/slog-common v0.17.1/go.mod h1:mZSJhinB4aqHziR0SKPqpVZjJ0JO35JfH+dDIWqaCBk= -github.com/samber/slog-multi v1.2.4 h1:k9x3JAWKJFPKffx+oXZ8TasaNuorIW4tG+TXxkt6Ry4= -github.com/samber/slog-multi v1.2.4/go.mod h1:ACuZ5B6heK57TfMVkVknN2UZHoFfjCwRxR0Q2OXKHlo= -github.com/samber/slog-sampling v1.5.2 h1:HaQmRGmLkVsXlVHEgIzgvxc6JYkkppL/7GN1B1g5LQM= -github.com/samber/slog-sampling v1.5.2/go.mod h1:n2PVbLAFRx8Im0KFt9srRrO/8wi9FAiTNBV7FmhbAHY= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/iso/iso.go b/internal/iso/iso.go index b695165e..58ece978 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -3,10 +3,11 @@ package iso import ( "bytes" "context" + "crypto/rand" "errors" "fmt" "io" - "log/slog" + "math/big" "net" "net/http" "net/http/httputil" @@ -14,7 +15,6 @@ import ( "path" "path/filepath" "strings" - "sync/atomic" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -23,7 +23,7 @@ import ( ) const ( - defaultConsoles = "console=ttyS0 console=ttyAMA0 console=tty1 console=tty0, console=ttyS1,115200" + defaultConsoles = "console=ttyS0 console=ttyAMA0 console=tty0, console=ttyS1 console=tty1" ) type Handler struct { @@ -43,16 +43,20 @@ type Handler struct { // in the source iso before patching. The field can be set // during build time by setting this field. // Ref: https://github.com/tinkerbell/hook/blob/main/linuxkit-templates/hook.template.yaml - MagicString string - SampleLogger *slog.Logger + MagicString string } -var total atomic.Int64 +func randomPercentage(precision int64) (float64, error) { + random, err := rand.Int(rand.Reader, big.NewInt(precision)) + if err != nil { + return 0, err + } + + return float64(random.Int64()) / float64(precision), nil +} func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { - log := h.Logger.WithValues("method", req.Method, "urlPath", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr, "total", total.Load()) - splog := h.SampleLogger.With("method", req.Method, "urlPath", req.URL.Path, "xff", req.Header.Get("X-Forwarded-For"), "remoteAddr", req.RemoteAddr, "total", total.Load()) - total.Add(1) + log := h.Logger.WithValues("method", req.Method, "urlPath", req.URL.Path, "remoteAddr", req.RemoteAddr) log.V(1).Info("starting the ISO patching HTTP handler") if req.Method != http.MethodHead && req.Method != http.MethodGet { return &http.Response{ @@ -140,7 +144,13 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } - splog.Info("HTTP GET method received with a 206 status code") + // 0.002% of the time we log a 206 request message. + // In testing, it was observed that about 3000 HTTP 206 requests are made per ISO mount. + // 0.002% gives us about 5, +/- 3, log messages per ISO mount. + // We're optimizing for showing "enough" log messages so that progress can be observed. + if p, _ := randomPercentage(10000); p < 0.002 { + log.Info("HTTP GET method received with a 206 status code") + } // this roundtripper func should only return error when there is no response from the server. // for any other case we log the error and return a 500 response diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index f2325f37..63f878d4 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -1,14 +1,9 @@ package iso import ( - "log/slog" "net/http" "net/url" - "os" "testing" - - slogmulti "github.com/samber/slog-multi" - slogsampling "github.com/samber/slog-sampling" ) func TestReqPathInvalid(t *testing.T) { @@ -23,20 +18,8 @@ func TestReqPathInvalid(t *testing.T) { for name, tt := range tests { u, _ := url.Parse(tt.isoURL) t.Run(name, func(t *testing.T) { - // Will print 10% of entries. - option := slogsampling.UniformSamplingOption{ - // The sample rate for sampling traces in the range [0.0, 1.0]. - Rate: 0.002, - } - - logger := slog.New( - slogmulti. - Pipe(option.NewMiddleware()). - Handler(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})), - ) h := &Handler{ - parsedURL: u, - SampleLogger: logger, + parsedURL: u, } req := http.Request{ Method: http.MethodGet, From 984e4ba39cf5d219b2bb92e2805db810774e2218 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 14:44:26 -0700 Subject: [PATCH 06/21] Add missing import Signed-off-by: Jacob Weinstock --- cmd/smee/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 814c2890..6c839130 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "log/slog" "net" "net/netip" "net/url" From b7a3db343356449ce3fac9c2764bc0036c807701 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 16:10:34 -0700 Subject: [PATCH 07/21] Remove returning on 200 status codes: This is needed for clients that request the entire ISO in one request as opposed to the 206 partial content requests. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 58ece978..ee6a37e2 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -23,7 +23,7 @@ import ( ) const ( - defaultConsoles = "console=ttyS0 console=ttyAMA0 console=tty0, console=ttyS1 console=tty1" + defaultConsoles = "console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1" ) type Handler struct { @@ -139,11 +139,6 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } - if resp.StatusCode == http.StatusOK { - log.Info("HTTP GET method received with a 200 status code") - return resp, nil - } - // 0.002% of the time we log a 206 request message. // In testing, it was observed that about 3000 HTTP 206 requests are made per ISO mount. // 0.002% gives us about 5, +/- 3, log messages per ISO mount. From 99215d683251b1f67705e6dd355966b0fab4112e Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 16:12:01 -0700 Subject: [PATCH 08/21] WIP: Add test for ISO patching: This creates a test ISO, patches it and validates the patch. I need to clean it up a bit though. Signed-off-by: Jacob Weinstock --- go.mod | 1 + go.sum | 2 + internal/iso/iso_test.go | 152 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) diff --git a/go.mod b/go.mod index d832edb6..ee5fd62b 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/go-logr/stdr v1.2.2 github.com/google/go-cmp v0.6.0 github.com/insomniacslk/dhcp v0.0.0-20240829085014-a3a4c1f04475 + github.com/kdomanski/iso9660 v0.4.0 github.com/packethost/xff v0.0.0-20190305172552-d3e9190c41b3 github.com/peterbourgon/ff/v3 v3.4.0 github.com/prometheus/client_golang v1.20.5 diff --git a/go.sum b/go.sum index 14f282aa..957389e6 100644 --- a/go.sum +++ b/go.sum @@ -86,6 +86,8 @@ github.com/josharian/native v1.1.0 h1:uuaP0hAbW7Y4l0ZRQ6C9zfb7Mg1mbFKry/xzDAfmtL github.com/josharian/native v1.1.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/kdomanski/iso9660 v0.4.0 h1:BPKKdcINz3m0MdjIMwS0wx1nofsOjxOq8TOr45WGHFg= +github.com/kdomanski/iso9660 v0.4.0/go.mod h1:OxUSupHsO9ceI8lBLPJKWBTphLemjrCQY8LPXM7qSzU= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 63f878d4..e862278c 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -1,11 +1,26 @@ package iso import ( + "context" + "io" + "log/slog" + "net" "net/http" + "net/http/httptest" "net/url" + "os" + "strings" "testing" + + "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" + "github.com/kdomanski/iso9660" + "github.com/kdomanski/iso9660/util" + "github.com/tinkerbell/smee/internal/dhcp/data" ) +const magicString = `464vn90e7rbj08xbwdjejmdf4it17c5zfzjyfhthbh19eij201hjgit021bmpdb9ctrc87x2ymc8e7icu4ffi15x1hah9iyaiz38ckyap8hwx2vt5rm44ixv4hau8iw718q5yd019um5dt2xpqqa2rjtdypzr5v1gun8un110hhwp8cex7pqrh2ivh0ynpm4zkkwc8wcn367zyethzy7q8hzudyeyzx3cgmxqbkh825gcak7kxzjbgjajwizryv7ec1xm2h0hh7pz29qmvtgfjj1vphpgq1zcbiiehv52wrjy9yq473d9t1rvryy6929nk435hfx55du3ih05kn5tju3vijreru1p6knc988d4gfdz28eragvryq5x8aibe5trxd0t6t7jwxkde34v6pj1khmp50k6qqj3nzgcfzabtgqkmeqhdedbvwf3byfdma4nkv3rcxugaj2d0ru30pa2fqadjqrtjnv8bu52xzxv7irbhyvygygxu1nt5z4fh9w1vwbdcmagep26d298zknykf2e88kumt59ab7nq79d8amnhhvbexgh48e8qc61vq2e9qkihzt1twk1ijfgw70nwizai15iqyted2dt9gfmf2gg7amzufre79hwqkddc1cd935ywacnkrnak6r7xzcz7zbmq3kt04u2hg1iuupid8rt4nyrju51e6uejb2ruu36g9aibmz3hnmvazptu8x5tyxk820g2cdpxjdij766bt2n3djur7v623a2v44juyfgz80ekgfb9hkibpxh3zgknw8a34t4jifhf116x15cei9hwch0fye3xyq0acuym8uhitu5evc4rag3ui0fny3qg4kju7zkfyy8hwh537urd5uixkzwu5bdvafz4jmv7imypj543xg5em8jk8cgk7c4504xdd5e4e71ihaumt6u5u2t1w7um92fepzae8p0vq93wdrd1756npu1pziiur1payc7kmdwyxg3hj5n4phxbc29x0tcddamjrwt260b0w` + func TestReqPathInvalid(t *testing.T) { tests := map[string]struct { isoURL string @@ -37,3 +52,140 @@ func TestReqPathInvalid(t *testing.T) { }) } } + +func TestPatching(t *testing.T) { + // create a small ISO file with the magic string + // serve it with a http server + // patch the ISO file + // mount the ISO file and check if the magic string was patched + + wantGrubCfg := `set timeout=0 +set gfxpayload=text +menuentry 'LinuxKit ISO Image' { + linuxefi /kernel facility=test console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1 syslog_host=127.0.0.1:514 grpc_authority=127.0.0.1:42113 tinkerbell_tls=false worker_id=de:ed:be:ef:fe:ed text + initrdefi /initrd.img +} +` + + // create a small ISO file with the magic string + grubCfg := `set timeout=0 +set gfxpayload=text +menuentry 'LinuxKit ISO Image' { + linuxefi /kernel 464vn90e7rbj08xbwdjejmdf4it17c5zfzjyfhthbh19eij201hjgit021bmpdb9ctrc87x2ymc8e7icu4ffi15x1hah9iyaiz38ckyap8hwx2vt5rm44ixv4hau8iw718q5yd019um5dt2xpqqa2rjtdypzr5v1gun8un110hhwp8cex7pqrh2ivh0ynpm4zkkwc8wcn367zyethzy7q8hzudyeyzx3cgmxqbkh825gcak7kxzjbgjajwizryv7ec1xm2h0hh7pz29qmvtgfjj1vphpgq1zcbiiehv52wrjy9yq473d9t1rvryy6929nk435hfx55du3ih05kn5tju3vijreru1p6knc988d4gfdz28eragvryq5x8aibe5trxd0t6t7jwxkde34v6pj1khmp50k6qqj3nzgcfzabtgqkmeqhdedbvwf3byfdma4nkv3rcxugaj2d0ru30pa2fqadjqrtjnv8bu52xzxv7irbhyvygygxu1nt5z4fh9w1vwbdcmagep26d298zknykf2e88kumt59ab7nq79d8amnhhvbexgh48e8qc61vq2e9qkihzt1twk1ijfgw70nwizai15iqyted2dt9gfmf2gg7amzufre79hwqkddc1cd935ywacnkrnak6r7xzcz7zbmq3kt04u2hg1iuupid8rt4nyrju51e6uejb2ruu36g9aibmz3hnmvazptu8x5tyxk820g2cdpxjdij766bt2n3djur7v623a2v44juyfgz80ekgfb9hkibpxh3zgknw8a34t4jifhf116x15cei9hwch0fye3xyq0acuym8uhitu5evc4rag3ui0fny3qg4kju7zkfyy8hwh537urd5uixkzwu5bdvafz4jmv7imypj543xg5em8jk8cgk7c4504xdd5e4e71ihaumt6u5u2t1w7um92fepzae8p0vq93wdrd1756npu1pziiur1payc7kmdwyxg3hj5n4phxbc29x0tcddamjrwt260b0w text + initrdefi /initrd.img +} +` + writer, err := iso9660.NewWriter() + if err != nil { + t.Fatal(err) + } + defer writer.Cleanup() + + f := strings.NewReader(grubCfg) + + fileToPatch := "EFI/BOOT/grub.cfg" + if err := writer.AddFile(f, fileToPatch); err != nil { + t.Fatal(err) + } + + outputFile, err := os.OpenFile("output.iso", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644) + if err != nil { + t.Fatalf("failed to create file: %s", err) + } + + defer os.Remove("output.iso") + + if err := writer.WriteTo(outputFile, "testvol"); err != nil { + t.Fatalf("failed to write ISO image: %s", err) + } + if err := outputFile.Close(); err != nil { + t.Fatalf("failed to close output file: %s", err) + } + + // serve it with a http server + hs := httptest.NewServer(http.FileServer(http.Dir("."))) + defer hs.Close() + + // patch the ISO file + u := hs.URL + "/output.iso" + parsedURL, err := url.Parse(u) + if err != nil { + t.Fatal(err) + } + + h := &Handler{ + Logger: logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelDebug})), + Backend: &mockBackend{}, + SourceISO: u, + ExtraKernelParams: []string{}, + Syslog: "127.0.0.1:514", + TinkServerTLS: false, + TinkServerGRPCAddr: "127.0.0.1:42113", + parsedURL: parsedURL, + MagicString: magicString, + } + + rurl := hs.URL + "/iso/de:ed:be:ef:fe:ed/output.iso" + purl, _ := url.Parse(rurl) + req := http.Request{ + Header: http.Header{}, + Method: http.MethodGet, + URL: purl, + } + res, err := h.RoundTrip(&req) + if err != nil { + t.Fatal(err) + } + if res.StatusCode != http.StatusOK { + t.Fatalf("got status code: %d, want status code: %d", res.StatusCode, http.StatusOK) + } + + isoContents, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile("patched.iso", isoContents, 0644); err != nil { + t.Fatal(err) + } + defer os.Remove("patched.iso") + + op, err := os.Open("patched.iso") + if err != nil { + t.Fatal(err) + } + defer op.Close() + os.Mkdir("mnt", 0755) + defer os.RemoveAll("mnt") + + // mount the ISO file and check if the magic string was patched + if err := util.ExtractImageToDirectory(op, "./mnt"); err != nil { + t.Fatal(err) + } + + grubCfgFile, err := os.ReadFile("./mnt/efi/boot/grub.cfg") + if err != nil { + t.Fatal(err) + } + t.Log(string(grubCfgFile)) + if diff := cmp.Diff(wantGrubCfg, string(grubCfgFile)); diff != "" { + t.Fatalf("unexpected grub.cfg file: %s", diff) + } +} + +type mockBackend struct{} + +func (m *mockBackend) GetByMac(context.Context, net.HardwareAddr) (*data.DHCP, *data.Netboot, error) { + d := &data.DHCP{} + n := &data.Netboot{ + Facility: "test", + } + return d, n, nil +} + +func (m *mockBackend) GetByIP(context.Context, net.IP) (*data.DHCP, *data.Netboot, error) { + d := &data.DHCP{} + n := &data.Netboot{ + Facility: "test", + } + return d, n, nil +} From 36a66d9ae44c975c11f4b329e563edcbb2dfc735 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 17:23:06 -0700 Subject: [PATCH 09/21] Use go-diskfs for ISO creation and reading: This library seems more maintained and allows for reading files in an ISO without having to mount it to the disk. Signed-off-by: Jacob Weinstock --- go.mod | 6 +++ go.sum | 19 ++++++++ internal/iso/iso.go | 3 +- internal/iso/iso_test.go | 102 ++++++++++++++++++++++++--------------- 4 files changed, 90 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index ee5fd62b..4a8264be 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ toolchain go1.23.2 require ( github.com/ccoveille/go-safecast v1.2.0 + github.com/diskfs/go-diskfs v1.4.2 github.com/fsnotify/fsnotify v1.8.0 github.com/ghodss/yaml v1.0.0 github.com/go-logr/logr v1.4.2 @@ -39,6 +40,8 @@ require ( github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect + github.com/djherbis/times v1.6.0 // indirect + github.com/elliotwutingfeng/asciiset v0.0.0-20230602022725-51bbb787efab // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect @@ -75,12 +78,15 @@ require ( github.com/pierrec/lz4/v4 v4.1.19 // indirect github.com/pin/tftp/v3 v3.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pkg/xattr v0.4.9 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/rs/zerolog v1.33.0 // indirect + github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect + github.com/ulikunitz/xz v0.5.11 // indirect github.com/vishvananda/netns v0.0.4 // indirect github.com/x448/float16 v0.8.4 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.32.0 // indirect diff --git a/go.sum b/go.sum index 957389e6..152deafd 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,12 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/diskfs/go-diskfs v1.4.2 h1:khBr9RTkqAZFaMYK7PP8NooL30hqj3bSgRmj3Ouguls= +github.com/diskfs/go-diskfs v1.4.2/go.mod h1:ss1uAUBhgDdEOewZFDWWpYqJFjNPbK7hYSjRoQE+D94= +github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c= +github.com/djherbis/times v1.6.0/go.mod h1:gOHeRAz2h+VJNZ5Gmc/o7iD9k4wW7NMVqieYCY99oc0= +github.com/elliotwutingfeng/asciiset v0.0.0-20230602022725-51bbb787efab h1:h1UgjJdAAhj+uPL68n7XASS6bU+07ZX1WJvVS2eyoeY= +github.com/elliotwutingfeng/asciiset v0.0.0-20230602022725-51bbb787efab/go.mod h1:GLo/8fDswSAniFG+BFIaiSPcK610jyzgEhWYPQwuQdw= github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= @@ -52,6 +58,8 @@ github.com/go-playground/validator/v10 v10.22.1 h1:40JcKH+bBNGFczGuoBYgX4I6m/i27 github.com/go-playground/validator/v10 v10.22.1/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= +github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= @@ -136,6 +144,8 @@ github.com/pin/tftp/v3 v3.1.0 h1:rQaxd4pGwcAJnpId8zC+O2NX3B2/NscjDZQaqEjuE7c= github.com/pin/tftp/v3 v3.1.0/go.mod h1:xwQaN4viYL019tM4i8iecm++5cGxSqen6AJEOEyEI0w= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/xattr v0.4.9 h1:5883YPCtkSd8LFbs13nXplj9g9tlrwoJRjgpgMu1/fE= +github.com/pkg/xattr v0.4.9/go.mod h1:di8WF84zAKk8jzR1UBTEWh9AUlIZZ7M/JNt8e9B6ktU= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -152,10 +162,13 @@ github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWN github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= +github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af h1:Sp5TG9f7K39yfB+If0vjp97vuT74F72r8hfRpP8jLU0= +github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tinkerbell/ipxedust v0.0.0-20241108174245-aa0c0298057d h1:MjxkPQbW7jGCgjMCjeS0Hs/o4yXqynEBDv1mUcFF+JI= @@ -164,6 +177,8 @@ github.com/tinkerbell/tink v0.12.1 h1:5ZCiGY1te59Qz/udFlzjh1UwKtmFBPOgo/LSK0J9Jy github.com/tinkerbell/tink v0.12.1/go.mod h1:H4w56sG0rMsEgHB3rBpW8/6KWKAvvAQWYHuZFpURkoU= github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 h1:YcojQL98T/OO+rybuzn2+5KrD5dBwXIvYBvQ2cD3Avg= github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63/go.mod h1:eLL9Nub3yfAho7qB0MzZizFhTU2QkLeoVsWdHtDW264= +github.com/ulikunitz/xz v0.5.11 h1:kpFauv27b6ynzBNT/Xy+1k+fK4WswhN/6PN5WhFAGw8= +github.com/ulikunitz/xz v0.5.11/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/vishvananda/netlink v1.3.0 h1:X7l42GfcV4S6E4vHTsw48qbrV+9PVojNfIhZcwQdrZk= github.com/vishvananda/netlink v1.3.0/go.mod h1:i6NetklAujEcC6fK0JPjT8qSwWyO0HLn4UKG+hGqeJs= github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8= @@ -220,7 +235,10 @@ golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20220408201424-a24fb2fb8a0f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220615213510-4f61da869c0c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -266,6 +284,7 @@ gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.31.2 h1:3wLBbL5Uom/8Zy98GRPXpJ254nEFpl+hwndmk9RwmL0= diff --git a/internal/iso/iso.go b/internal/iso/iso.go index ee6a37e2..ab66dc1f 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -148,7 +148,8 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { } // this roundtripper func should only return error when there is no response from the server. - // for any other case we log the error and return a 500 response + // for any other case we log the error and return a 500 response. See the http.RoundTripper interface code + // comments for more details. b, err := io.ReadAll(resp.Body) if err != nil { log.Error(err, "issue reading response bytes") diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index e862278c..b3b2383b 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -2,6 +2,7 @@ package iso import ( "context" + "fmt" "io" "log/slog" "net" @@ -9,13 +10,14 @@ import ( "net/http/httptest" "net/url" "os" - "strings" "testing" + diskfs "github.com/diskfs/go-diskfs" + "github.com/diskfs/go-diskfs/disk" + "github.com/diskfs/go-diskfs/filesystem" + "github.com/diskfs/go-diskfs/filesystem/iso9660" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" - "github.com/kdomanski/iso9660" - "github.com/kdomanski/iso9660/util" "github.com/tinkerbell/smee/internal/dhcp/data" ) @@ -53,21 +55,8 @@ func TestReqPathInvalid(t *testing.T) { } } -func TestPatching(t *testing.T) { - // create a small ISO file with the magic string - // serve it with a http server - // patch the ISO file - // mount the ISO file and check if the magic string was patched - - wantGrubCfg := `set timeout=0 -set gfxpayload=text -menuentry 'LinuxKit ISO Image' { - linuxefi /kernel facility=test console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1 syslog_host=127.0.0.1:514 grpc_authority=127.0.0.1:42113 tinkerbell_tls=false worker_id=de:ed:be:ef:fe:ed text - initrdefi /initrd.img -} -` - - // create a small ISO file with the magic string +func TestCreateISO2(t *testing.T) { + t.Skip("Unskip this test to create a new ISO file") grubCfg := `set timeout=0 set gfxpayload=text menuentry 'LinuxKit ISO Image' { @@ -75,35 +64,63 @@ menuentry 'LinuxKit ISO Image' { initrdefi /initrd.img } ` - writer, err := iso9660.NewWriter() + if err := os.Remove("testdata/output.iso"); err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + var diskSize int64 = 51200 // 50Kb + mydisk, err := diskfs.Create("./testdata/output.iso", diskSize, diskfs.Raw, diskfs.SectorSizeDefault) if err != nil { t.Fatal(err) } - defer writer.Cleanup() + defer mydisk.Close() - f := strings.NewReader(grubCfg) - - fileToPatch := "EFI/BOOT/grub.cfg" - if err := writer.AddFile(f, fileToPatch); err != nil { + // the following line is required for an ISO, which may have logical block sizes + // only of 2048, 4096, 8192 + mydisk.LogicalBlocksize = 2048 + fspec := disk.FilesystemSpec{Partition: 0, FSType: filesystem.TypeISO9660, VolumeLabel: "label"} + fs, err := mydisk.CreateFilesystem(fspec) + if err != nil { t.Fatal(err) } - - outputFile, err := os.OpenFile("output.iso", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644) + if err := fs.Mkdir("EFI/BOOT"); err != nil { + t.Fatal(err) + } + rw, err := fs.OpenFile("EFI/BOOT/grub.cfg", os.O_CREATE|os.O_RDWR) if err != nil { - t.Fatalf("failed to create file: %s", err) + t.Fatal(err) } - - defer os.Remove("output.iso") - - if err := writer.WriteTo(outputFile, "testvol"); err != nil { - t.Fatalf("failed to write ISO image: %s", err) + content := []byte(grubCfg) + _, err = rw.Write(content) + if err != nil { + t.Fatal(err) + } + iso, ok := fs.(*iso9660.FileSystem) + if !ok { + t.Fatal(fmt.Errorf("not an iso9660 filesystem")) } - if err := outputFile.Close(); err != nil { - t.Fatalf("failed to close output file: %s", err) + err = iso.Finalize(iso9660.FinalizeOptions{}) + if err != nil { + t.Fatal(err) } +} + +func TestPatching(t *testing.T) { + // create a small ISO file with the magic string + // serve ISO with a http server + // patch the ISO file + // mount the ISO file and check if the magic string was patched + + wantGrubCfg := `set timeout=0 +set gfxpayload=text +menuentry 'LinuxKit ISO Image' { + linuxefi /kernel facility=test console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1 syslog_host=127.0.0.1:514 grpc_authority=127.0.0.1:42113 tinkerbell_tls=false worker_id=de:ed:be:ef:fe:ed text + initrdefi /initrd.img +} +` + // This expects that testdata/output.iso exists. Run the TestCreateISO test to create it. // serve it with a http server - hs := httptest.NewServer(http.FileServer(http.Dir("."))) + hs := httptest.NewServer(http.FileServer(http.Dir("./testdata"))) defer hs.Close() // patch the ISO file @@ -157,15 +174,22 @@ menuentry 'LinuxKit ISO Image' { os.Mkdir("mnt", 0755) defer os.RemoveAll("mnt") - // mount the ISO file and check if the magic string was patched - if err := util.ExtractImageToDirectory(op, "./mnt"); err != nil { + dd, err := diskfs.Open("patched.iso", diskfs.WithOpenMode(diskfs.ReadOnly)) + if err != nil { t.Fatal(err) } - - grubCfgFile, err := os.ReadFile("./mnt/efi/boot/grub.cfg") + defer dd.Close() + fs, err := dd.GetFilesystem(0) + if err != nil { + t.Fatal(err) + } + ff, err := fs.OpenFile("EFI/BOOT/GRUB.CFG", os.O_RDONLY) if err != nil { t.Fatal(err) } + defer ff.Close() + grubCfgFile, err := io.ReadAll(ff) + t.Log(string(grubCfgFile)) if diff := cmp.Diff(wantGrubCfg, string(grubCfgFile)); diff != "" { t.Fatalf("unexpected grub.cfg file: %s", diff) From fb3d86d6830ae983139172af18a267d30181b991 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 17:25:38 -0700 Subject: [PATCH 10/21] Don't log in ISO patch test: This adds logging output to the test results and makes them more difficult to read. Signed-off-by: Jacob Weinstock --- internal/iso/iso_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index b3b2383b..4e4f6ed8 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "log/slog" "net" "net/http" "net/http/httptest" @@ -131,7 +130,7 @@ menuentry 'LinuxKit ISO Image' { } h := &Handler{ - Logger: logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelDebug})), + Logger: logr.Discard(), //logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelInfo})), Backend: &mockBackend{}, SourceISO: u, ExtraKernelParams: []string{}, @@ -189,8 +188,10 @@ menuentry 'LinuxKit ISO Image' { } defer ff.Close() grubCfgFile, err := io.ReadAll(ff) + if err != nil { + t.Fatal(err) + } - t.Log(string(grubCfgFile)) if diff := cmp.Diff(wantGrubCfg, string(grubCfgFile)); diff != "" { t.Fatalf("unexpected grub.cfg file: %s", diff) } From c1dfc8ba6fa526405cdd16284f9687b3a40e7be1 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 17:29:14 -0700 Subject: [PATCH 11/21] Clean up unused code: Signed-off-by: Jacob Weinstock --- internal/iso/iso_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 4e4f6ed8..42fc3d9e 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -130,7 +130,7 @@ menuentry 'LinuxKit ISO Image' { } h := &Handler{ - Logger: logr.Discard(), //logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelInfo})), + Logger: logr.Discard(), Backend: &mockBackend{}, SourceISO: u, ExtraKernelParams: []string{}, @@ -152,6 +152,7 @@ menuentry 'LinuxKit ISO Image' { if err != nil { t.Fatal(err) } + defer res.Body.Close() if res.StatusCode != http.StatusOK { t.Fatalf("got status code: %d, want status code: %d", res.StatusCode, http.StatusOK) } @@ -160,19 +161,11 @@ menuentry 'LinuxKit ISO Image' { if err != nil { t.Fatal(err) } - if err := os.WriteFile("patched.iso", isoContents, 0644); err != nil { + if err := os.WriteFile("patched.iso", isoContents, 0o644); err != nil { t.Fatal(err) } defer os.Remove("patched.iso") - op, err := os.Open("patched.iso") - if err != nil { - t.Fatal(err) - } - defer op.Close() - os.Mkdir("mnt", 0755) - defer os.RemoveAll("mnt") - dd, err := diskfs.Open("patched.iso", diskfs.WithOpenMode(diskfs.ReadOnly)) if err != nil { t.Fatal(err) From d959b39a73ede40d94a136ea2d1a3dd815e454c9 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 17:36:42 -0700 Subject: [PATCH 12/21] Clean up go.mod and go.sum: Signed-off-by: Jacob Weinstock --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index 4a8264be..188a08f0 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,6 @@ require ( github.com/go-logr/stdr v1.2.2 github.com/google/go-cmp v0.6.0 github.com/insomniacslk/dhcp v0.0.0-20240829085014-a3a4c1f04475 - github.com/kdomanski/iso9660 v0.4.0 github.com/packethost/xff v0.0.0-20190305172552-d3e9190c41b3 github.com/peterbourgon/ff/v3 v3.4.0 github.com/prometheus/client_golang v1.20.5 diff --git a/go.sum b/go.sum index 152deafd..067a4820 100644 --- a/go.sum +++ b/go.sum @@ -94,8 +94,6 @@ github.com/josharian/native v1.1.0 h1:uuaP0hAbW7Y4l0ZRQ6C9zfb7Mg1mbFKry/xzDAfmtL github.com/josharian/native v1.1.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= -github.com/kdomanski/iso9660 v0.4.0 h1:BPKKdcINz3m0MdjIMwS0wx1nofsOjxOq8TOr45WGHFg= -github.com/kdomanski/iso9660 v0.4.0/go.mod h1:OxUSupHsO9ceI8lBLPJKWBTphLemjrCQY8LPXM7qSzU= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= From 82aff58237a7a296e99fa7c774858df5eba2c7c0 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 17:44:14 -0700 Subject: [PATCH 13/21] Add small ISO for testing: Signed-off-by: Jacob Weinstock --- internal/iso/testdata/output.iso | Bin 0 -> 51200 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 internal/iso/testdata/output.iso diff --git a/internal/iso/testdata/output.iso b/internal/iso/testdata/output.iso new file mode 100644 index 0000000000000000000000000000000000000000..13c76e3654cb943afcb9bc274960345211086e34 GIT binary patch literal 51200 zcmeI(%dXtE83*tg+l%02RohLn92^wQCiM|1QI{53G-+ZOX=gYqt%>o5ClPciUNk=_&>mi91W?y^^!uk@-Bs|DOM%zPe=fbR@pyh8KKS5peJOVR%O8q=bk1HM9$w}= zRF_o!%*1ybuZ>P$+@zrl{ZaVFhZ=Ud%{f*GCZ2Rh_d-@H?ZMYBm^J;fu9Qe>wk>@Kdp&55P$##AOHafysH9t zkFoD8as1Wc(Pj4^nM-PZJWjv=)z{D7RUd3O1Rwwb2teTHC~$p8>^|Oo75@0^!{ZCh z=Oha009U<;2jmXz9V*@a&`XHwPCE|i5C*ruxY$2T=NiD7B6ym!TM^Sy8>HBo1Sr zZPu>5T=|gZ#TKPgTP&thZC*qfw~+F#4^v}RNm<8hTiCFfrZcr!^)|<)keq~`s(tar zxY26r7uU7MkY!Y3H<67uT1&Msy3Qt6amtERRF2Hc9AjS5YOq<#R2vu*KGiB=c3{r< zbkSy(vnOM22VRw>$huZ+w3Kt*?R6&>%33#)xjBf$<*qPmutKKUl8=U1)sHN(`81hy z*gD>Bb>q6p_Ht4}ie_2cw27_g>uj(~I`iPW+E9_qK264&wo@(4G&?gaW#btQTUZJ` zH!3c%i(T+!aBRz3Rjp)M8^O=*WTd&ERk_(A+Lp?)w7z$&s+(e%ib|AqKPa^{GIS~{ zWjXPjEEAR0WVPBHajPZTwB1@H!;V|;tw2xFNkN6Hy0&j=*A;cY`F55?rPj#`!3h^t z%Hmq{!OlZ%^SKBc--@m2r_{P6^Hg<2dp~MX&Mq6`Y&;_}_tG?U_CC$KYKQE%R1N)7 zZ=>^NU@it*(xjtYjKK(Piac)|9a3TZTr4?FYr15a@v-f;GLg1xn@U;Tj6tPaXNR?{ z)68W?x2h#14}@{4_l@Q%X`@U-oRXUR*q2^u=b2o1wjbEk>!clGI(3;fzSwLV=5?^!>RBU}y4^C<@5iK%qhVPZIwt$l*phd);2BG@AOw?{EQsFEl-}i@dFseo z_`ag89JgAQW3o(Dx{6tlg5|@wx3*1dKa;(0;DznQ8bg;V!-i~Bq2aU&iQ_`ly_r`> z^Q1}E({2CpefOXT?PhV?KU}g;bl+Y5^Xjd3e^@yLAOHafKmY;|fB*#ED}jFlj Date: Tue, 19 Nov 2024 18:15:01 -0700 Subject: [PATCH 14/21] Don't return error from randomPercentage: I was just ignoring the error, so an error is now just returned as 0. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index ab66dc1f..6b63177e 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -46,13 +46,13 @@ type Handler struct { MagicString string } -func randomPercentage(precision int64) (float64, error) { +func randomPercentage(precision int64) float64 { random, err := rand.Int(rand.Reader, big.NewInt(precision)) if err != nil { - return 0, err + return 0 } - return float64(random.Int64()) / float64(precision), nil + return float64(random.Int64()) / float64(precision) } func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { @@ -143,7 +143,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // In testing, it was observed that about 3000 HTTP 206 requests are made per ISO mount. // 0.002% gives us about 5, +/- 3, log messages per ISO mount. // We're optimizing for showing "enough" log messages so that progress can be observed. - if p, _ := randomPercentage(10000); p < 0.002 { + if p := randomPercentage(10000); p < 0.002 { log.Info("HTTP GET method received with a 206 status code") } From 50d4f3af8f03f31cfd32beeae09b82a0f325f5d5 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 19:13:07 -0700 Subject: [PATCH 15/21] Fix test name: Signed-off-by: Jacob Weinstock --- internal/iso/iso_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 42fc3d9e..7f848f2f 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -54,7 +54,7 @@ func TestReqPathInvalid(t *testing.T) { } } -func TestCreateISO2(t *testing.T) { +func TestCreateISO(t *testing.T) { t.Skip("Unskip this test to create a new ISO file") grubCfg := `set timeout=0 set gfxpayload=text From 608c5b3463efdb4e0cfd806aa01949daa23b0c52 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Tue, 19 Nov 2024 19:18:35 -0700 Subject: [PATCH 16/21] Get the grub.cfg contents from byte slice: This allows us to avoid having to write a file to disk in a unit test. Signed-off-by: Jacob Weinstock --- internal/iso/iso_test.go | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 7f848f2f..97c03735 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -1,6 +1,7 @@ package iso import ( + "bytes" "context" "fmt" "io" @@ -161,31 +162,14 @@ menuentry 'LinuxKit ISO Image' { if err != nil { t.Fatal(err) } - if err := os.WriteFile("patched.iso", isoContents, 0o644); err != nil { - t.Fatal(err) - } - defer os.Remove("patched.iso") - dd, err := diskfs.Open("patched.iso", diskfs.WithOpenMode(diskfs.ReadOnly)) - if err != nil { - t.Fatal(err) - } - defer dd.Close() - fs, err := dd.GetFilesystem(0) - if err != nil { - t.Fatal(err) - } - ff, err := fs.OpenFile("EFI/BOOT/GRUB.CFG", os.O_RDONLY) - if err != nil { - t.Fatal(err) - } - defer ff.Close() - grubCfgFile, err := io.ReadAll(ff) - if err != nil { - t.Fatal(err) + idx := bytes.Index(isoContents, []byte(wantGrubCfg)) + if idx == -1 { + t.Fatalf("could not find grub.cfg in the ISO") } + contents := isoContents[idx : idx+len(wantGrubCfg)] - if diff := cmp.Diff(wantGrubCfg, string(grubCfgFile)); diff != "" { + if diff := cmp.Diff(wantGrubCfg, string(contents)); diff != "" { t.Fatalf("unexpected grub.cfg file: %s", diff) } } From 361c83d47fdf488ccb70fb247502326a691298ff Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Wed, 20 Nov 2024 23:57:13 -0700 Subject: [PATCH 17/21] WIP: Handle potentially dangerous range requests: This is a work in progress to handle ranges that could cause the patching to read into memory large chunk sizes. This could cause OOM killing and/or DOS the service. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 86 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 6b63177e..be75bd64 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -14,6 +14,7 @@ import ( "net/url" "path" "path/filepath" + "strconv" "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -56,7 +57,7 @@ func randomPercentage(precision int64) float64 { } func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { - log := h.Logger.WithValues("method", req.Method, "urlPath", req.URL.Path, "remoteAddr", req.RemoteAddr) + log := h.Logger.WithValues("method", req.Method, "urlPath", req.URL.Path, "remoteAddr", req.RemoteAddr, "fullURL", req.URL.String()) log.V(1).Info("starting the ISO patching HTTP handler") if req.Method != http.MethodHead && req.Method != http.MethodGet { return &http.Response{ @@ -125,19 +126,89 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { log.Info("issue getting the source ISO", "sourceIso", h.SourceISO, "error", err) return nil, err } + // by setting this header we are telling the logging middleware to not log its default log message. + // we do this because there are a lot of partial content requests. + resp.Header.Set("X-Global-Logging", "false") + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { log.Info("the request to get the source ISO returned a status other than ok (200) or partial content (206)", "sourceIso", h.SourceISO, "status", resp.Status) return resp, nil } - // by setting this header we are telling the logging middleware to not log its default log message. - // we do this because there are a lot of partial content requests. - resp.Header.Set("X-Global-Logging", "false") if req.Method == http.MethodHead { // Fuse clients typically make a HEAD request before they start requesting content. - log.Info("HTTP HEAD method received") + log.Info("HTTP HEAD method received", "status", resp.Status) + return resp, nil + } + + // At this point we only allow HTTP GET method with a 206 status code. + // Otherwise we are potentially reading the entire ISO file and patching it. + // This is not the intended use case for this handler. + // And this can cause memory issues, like OOM, if the ISO file is too large. + // By returning the `resp` here we allow clients to download the ISO file but without any patching. + // This is done so that there can be a minimal amount of troubleshooting for SourceISO issues. + if resp.StatusCode != http.StatusPartialContent { + log.Info("HTTP GET method received with a status code other than 206, source iso will be unpatched", "status", resp.Status) return resp, nil } + // If the request is a partial content request, we need to validate the Content-Range header. + // Because we read the entire response body into memory for patching, we need to ensure that the + // Content-Range is within a reasonable size. For now, we are limiting the size to 500Kb. + + // Content range RFC: https://tools.ietf.org/html/rfc7233#section-4.2 + // https://datatracker.ietf.org/doc/html/rfc7233#section-4.4 + + // Get the content range from the response header + contentRange := resp.Header.Get("Content-Range") + l := strings.Split(contentRange, "/") + if len(l) == 2 { + sp := strings.Split(l[0], " ") + if len(sp) == 2 { + contentRange = fmt.Sprintf("%s=%s", sp[0], sp[1]) + } + } + ln := len(l) - 1 + size := l[ln] + sizeInt, err := strconv.ParseInt(size, 10, 64) + if err != nil { + log.Error(err, "unable to get the size of the content from the range header", "respContentRange", contentRange, "reqContentRange", resp.Request.Header.Get("Range")) + return &http.Response{ + Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), + StatusCode: http.StatusInternalServerError, + Body: http.NoBody, + Request: req, + Header: resp.Header, + }, nil + } + + crp, err := ParseRange(contentRange, sizeInt) + if err != nil { + log.Error(err, "issue parsing the content range header", "respContentRange", contentRange, "reqContentRange", resp.Request.Header.Get("Range")) + resp.Header.Set("Content-Range", fmt.Sprintf("bytes */%d", sizeInt)) + return &http.Response{ + Status: fmt.Sprintf("%d %s", http.StatusRequestedRangeNotSatisfiable, http.StatusText(http.StatusRequestedRangeNotSatisfiable)), + StatusCode: http.StatusRequestedRangeNotSatisfiable, + Body: http.NoBody, + Request: req, + Header: resp.Header, + }, nil + } + + for _, r := range crp { + if r.Length > 500*1024 { + log.Info("content range length is greater than 512Kb", "contentRange", contentRange) + resp.Header.Set("Content-Range", fmt.Sprintf("bytes */%d", sizeInt)) + return &http.Response{ + Status: fmt.Sprintf("%d %s", http.StatusRequestedRangeNotSatisfiable, http.StatusText(http.StatusRequestedRangeNotSatisfiable)), + StatusCode: http.StatusRequestedRangeNotSatisfiable, + Body: http.NoBody, + Request: req, + Header: resp.Header, + }, nil + } + } + // Check that it is less than 500Kb + //http.StatusRequestedRangeNotSatisfiable // 0.002% of the time we log a 206 request message. // In testing, it was observed that about 3000 HTTP 206 requests are made per ISO mount. @@ -152,12 +223,15 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // comments for more details. b, err := io.ReadAll(resp.Body) if err != nil { + //var b []byte + //if _, err := io.LimitReader(resp.Body, 500*1024).Read(b); err != nil { log.Error(err, "issue reading response bytes") return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), StatusCode: http.StatusInternalServerError, Body: http.NoBody, Request: req, + Header: resp.Header, }, nil } if err := resp.Body.Close(); err != nil { @@ -167,6 +241,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { StatusCode: http.StatusInternalServerError, Body: http.NoBody, Request: req, + Header: resp.Header, }, nil } @@ -197,7 +272,6 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { } resp.Body = io.NopCloser(bytes.NewReader(b)) - log.V(1).Info("roundtrip complete") return resp, nil } From 4b18a166d839951c66f4d1e179f4fca6010690d5 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Wed, 20 Nov 2024 23:59:07 -0700 Subject: [PATCH 18/21] Update Tiltfile for faster iteration: The live update makes the development loop quite fast. Signed-off-by: Jacob Weinstock --- Tiltfile | 57 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/Tiltfile b/Tiltfile index 7f2d2c66..50754c22 100644 --- a/Tiltfile +++ b/Tiltfile @@ -2,32 +2,47 @@ local_resource('compile smee', cmd='make cmd/smee/smee-linux-amd64', deps=["go.mod", "go.sum", "internal", "Dockerfile", "cmd/smee/main.go", "cmd/smee/flag.go", "cmd/smee/backend.go"] ) -docker_build( - 'quay.io/tinkerbell/smee', - '.', - dockerfile='Dockerfile', +load('ext://restart_process', 'docker_build_with_restart') +#docker_build( +# 'quay.io/tinkerbell/smee', +# '.', +# dockerfile='Dockerfile', +#) +docker_build_with_restart( + 'quay.io/tinkerbell/smee', + '.', + dockerfile='Dockerfile', + entrypoint=['/usr/bin/smee'], + live_update=[ + sync('cmd/smee/smee-linux-amd64', '/usr/bin/smee'), + ], ) -#k8s_yaml(kustomize('./manifests/kustomize/overlays/k3d')) default_registry('ttl.sh/meohmy-dghentld') -trusted_proxies = os.getenv('trusted_proxies', '') -lb_ip = os.getenv('LB_IP', '192.168.2.114') +load('ext://local_output', 'local_output') +default_trusted_proxies = local_output("kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ','") +trusted_proxies = os.getenv('TRUSTED_PROXIES', default_trusted_proxies) +lb_ip = os.getenv('LB_IP', '') stack_version = os.getenv('STACK_CHART_VERSION', '0.5.0') -layer2_interface = os.getenv('LAYER2_INTERFACE', 'eth1') +stack_location = os.getenv('STACK_LOCATION', '/home/tink/repos/tinkerbell/charts/tinkerbell/stack') # or a local path like '/home/tink/repos/tinkerbell/charts/tinkerbell/stack' +namespace = 'tink' + +if lb_ip == '': + fail('Please set the LB_IP environment variable. This is required to deploy the stack.') load('ext://helm_resource', 'helm_resource') helm_resource('stack', - chart='oci://ghcr.io/tinkerbell/charts/stack', - namespace='tink', - image_deps=['quay.io/tinkerbell/smee'], - image_keys=[('smee.image')], - flags=[ - '--create-namespace', - '--version=%s' % stack_version, - '--set=global.trustedProxies={%s}' % trusted_proxies, - '--set=global.publicIP=%s' % lb_ip, - '--set=stack.kubevip.interface=%s' % layer2_interface, - '--set=stack.relay.sourceInterface=%s' % layer2_interface, - ], - release_name='tink-stack' + chart=stack_location, + namespace=namespace, + image_deps=['quay.io/tinkerbell/smee'], + image_keys=[('smee.image')], + flags=[ + '--create-namespace', + '--version=%s' % stack_version, + '--set=global.trustedProxies={%s}' % trusted_proxies, + '--set=global.publicIP=%s' % lb_ip, + '--set=stack.kubevip.interface=eth1', + '--set=stack.relay.sourceInterface=eth1', + ], + release_name='stack' ) From c6e17f70318099de56bda0e33cb39402cac95d34 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 21 Nov 2024 11:12:29 -0700 Subject: [PATCH 19/21] Memory safety update: If the request is a partial content request, we need to validate the Content-Range header. Because we read the entire response body into memory for patching, we need to ensure that the Content-Range is within a reasonable size. For now, we are limiting the size to 500Kb. Signed-off-by: Jacob Weinstock --- Tiltfile | 10 +++-- cmd/smee/main.go | 2 +- internal/iso/iso.go | 88 ++++++++++------------------------------ internal/iso/iso_test.go | 8 ++-- 4 files changed, 34 insertions(+), 74 deletions(-) diff --git a/Tiltfile b/Tiltfile index 50754c22..b54d1e85 100644 --- a/Tiltfile +++ b/Tiltfile @@ -1,8 +1,12 @@ +load('ext://restart_process', 'docker_build_with_restart') +load('ext://local_output', 'local_output') +load('ext://helm_resource', 'helm_resource') + local_resource('compile smee', cmd='make cmd/smee/smee-linux-amd64', - deps=["go.mod", "go.sum", "internal", "Dockerfile", "cmd/smee/main.go", "cmd/smee/flag.go", "cmd/smee/backend.go"] + deps=["go.mod", "go.sum", "internal", "Dockerfile", "cmd/smee/main.go", "cmd/smee/flag.go", "cmd/smee/backend.go"], ) -load('ext://restart_process', 'docker_build_with_restart') + #docker_build( # 'quay.io/tinkerbell/smee', # '.', @@ -19,7 +23,6 @@ docker_build_with_restart( ) default_registry('ttl.sh/meohmy-dghentld') -load('ext://local_output', 'local_output') default_trusted_proxies = local_output("kubectl get nodes -o jsonpath='{.items[*].spec.podCIDR}' | tr ' ' ','") trusted_proxies = os.getenv('TRUSTED_PROXIES', default_trusted_proxies) lb_ip = os.getenv('LB_IP', '') @@ -30,7 +33,6 @@ namespace = 'tink' if lb_ip == '': fail('Please set the LB_IP environment variable. This is required to deploy the stack.') -load('ext://helm_resource', 'helm_resource') helm_resource('stack', chart=stack_location, namespace=namespace, diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 6c839130..94426659 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -269,7 +269,7 @@ func main() { return cfg.iso.magicString }(), } - isoHandler, err := ih.Reverse() + isoHandler, err := ih.HandlerFunc() if err != nil { panic(fmt.Errorf("failed to create iso handler: %w", err)) } diff --git a/internal/iso/iso.go b/internal/iso/iso.go index be75bd64..5ce49453 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -14,7 +14,6 @@ import ( "net/url" "path" "path/filepath" - "strconv" "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -24,7 +23,8 @@ import ( ) const ( - defaultConsoles = "console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1" + defaultConsoles = "console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1" + maxContentLength int64 = 500 * 1024 // 500Kb ) type Handler struct { @@ -113,10 +113,9 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - // Reverse Proxy modifies the request url to - // the same path it received the incoming request. - // mac-id is added to the url path to do hardware lookups using the backend reader - // and is not used when making http calls to the source url. + // The httputil.NewSingleHostReverseProxy takes the incoming request url and adds the path to the target (h.SourceISO). + // This function is more than a pass through proxy. The MAC address in the url path is required to do hardware lookups using the backend reader + // and is not used when making http calls to the target (h.SourceISO). All valid requests are passed through to the target. req.URL.Path = h.parsedURL.Path // RoundTripper needs a Transport to execute a HTTP transaction @@ -127,16 +126,18 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return nil, err } // by setting this header we are telling the logging middleware to not log its default log message. - // we do this because there are a lot of partial content requests. + // we do this because there are a lot of partial content requests and it allow this handler to take care of logging. resp.Header.Set("X-Global-Logging", "false") if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { + // This log line is not rate limited as we don't anticipate this to be a common occurrence or happen frequently when it does. log.Info("the request to get the source ISO returned a status other than ok (200) or partial content (206)", "sourceIso", h.SourceISO, "status", resp.Status) return resp, nil } if req.Method == http.MethodHead { - // Fuse clients typically make a HEAD request before they start requesting content. + // Fuse clients typically make a HEAD request before they start requesting content. This is not rate limited as the occurrence is expected to be low. + // This allows provides us some info on the progress of the client. log.Info("HTTP HEAD method received", "status", resp.Status) return resp, nil } @@ -148,83 +149,36 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // By returning the `resp` here we allow clients to download the ISO file but without any patching. // This is done so that there can be a minimal amount of troubleshooting for SourceISO issues. if resp.StatusCode != http.StatusPartialContent { - log.Info("HTTP GET method received with a status code other than 206, source iso will be unpatched", "status", resp.Status) + log.Info("HTTP GET method received with a status code other than 206, source iso will be unpatched", "status", resp.Status, "respHeader", resp.Header, "reqHeaders", resp.Request.Header) return resp, nil } // If the request is a partial content request, we need to validate the Content-Range header. // Because we read the entire response body into memory for patching, we need to ensure that the - // Content-Range is within a reasonable size. For now, we are limiting the size to 500Kb. + // Content-Range is within a reasonable size. For now, we are limiting the size to 500Kb (partialContentMax). // Content range RFC: https://tools.ietf.org/html/rfc7233#section-4.2 // https://datatracker.ietf.org/doc/html/rfc7233#section-4.4 // Get the content range from the response header - contentRange := resp.Header.Get("Content-Range") - l := strings.Split(contentRange, "/") - if len(l) == 2 { - sp := strings.Split(l[0], " ") - if len(sp) == 2 { - contentRange = fmt.Sprintf("%s=%s", sp[0], sp[1]) - } - } - ln := len(l) - 1 - size := l[ln] - sizeInt, err := strconv.ParseInt(size, 10, 64) - if err != nil { - log.Error(err, "unable to get the size of the content from the range header", "respContentRange", contentRange, "reqContentRange", resp.Request.Header.Get("Range")) - return &http.Response{ - Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), - StatusCode: http.StatusInternalServerError, - Body: http.NoBody, - Request: req, - Header: resp.Header, - }, nil - } - - crp, err := ParseRange(contentRange, sizeInt) - if err != nil { - log.Error(err, "issue parsing the content range header", "respContentRange", contentRange, "reqContentRange", resp.Request.Header.Get("Range")) - resp.Header.Set("Content-Range", fmt.Sprintf("bytes */%d", sizeInt)) - return &http.Response{ - Status: fmt.Sprintf("%d %s", http.StatusRequestedRangeNotSatisfiable, http.StatusText(http.StatusRequestedRangeNotSatisfiable)), - StatusCode: http.StatusRequestedRangeNotSatisfiable, - Body: http.NoBody, - Request: req, - Header: resp.Header, - }, nil - } - - for _, r := range crp { - if r.Length > 500*1024 { - log.Info("content range length is greater than 512Kb", "contentRange", contentRange) - resp.Header.Set("Content-Range", fmt.Sprintf("bytes */%d", sizeInt)) - return &http.Response{ - Status: fmt.Sprintf("%d %s", http.StatusRequestedRangeNotSatisfiable, http.StatusText(http.StatusRequestedRangeNotSatisfiable)), - StatusCode: http.StatusRequestedRangeNotSatisfiable, - Body: http.NoBody, - Request: req, - Header: resp.Header, - }, nil - } + if resp.ContentLength > maxContentLength { + log.Info("content length is greater than max", "contentLengthBytes", resp.ContentLength, "maxAllowedBytes", maxContentLength) + return resp, nil } - // Check that it is less than 500Kb - //http.StatusRequestedRangeNotSatisfiable // 0.002% of the time we log a 206 request message. // In testing, it was observed that about 3000 HTTP 206 requests are made per ISO mount. - // 0.002% gives us about 5, +/- 3, log messages per ISO mount. + // 0.002% gives us about 5 - 10, log messages per ISO mount. // We're optimizing for showing "enough" log messages so that progress can be observed. - if p := randomPercentage(10000); p < 0.002 { + if p := randomPercentage(100000); p < 0.002 { log.Info("HTTP GET method received with a 206 status code") } // this roundtripper func should only return error when there is no response from the server. // for any other case we log the error and return a 500 response. See the http.RoundTripper interface code // comments for more details. - b, err := io.ReadAll(resp.Body) - if err != nil { - //var b []byte - //if _, err := io.LimitReader(resp.Body, 500*1024).Read(b); err != nil { + var b []byte + respBuf := new(bytes.Buffer) + if _, err := io.CopyN(respBuf, resp.Body, resp.ContentLength); err != nil { log.Error(err, "issue reading response bytes") return &http.Response{ Status: fmt.Sprintf("%d %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), @@ -234,6 +188,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { Header: resp.Header, }, nil } + b = respBuf.Bytes() if err := resp.Body.Close(); err != nil { log.Error(err, "issue closing response body") return &http.Response{ @@ -276,7 +231,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } -func (h *Handler) Reverse() (http.HandlerFunc, error) { +func (h *Handler) HandlerFunc() (http.HandlerFunc, error) { target, err := url.Parse(h.SourceISO) if err != nil { return nil, err @@ -314,6 +269,7 @@ func getFacility(ctx context.Context, mac net.HardwareAddr, br handler.BackendRe return "", errors.New("backend is nil") } + // TODO(jacobweinstock): Pass DHCP info to kernel cmdline parameters for static IP assignment. _, n, err := br.GetByMac(ctx, mac) if err != nil { return "", err diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 97c03735..d35b485d 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io" + "log/slog" "net" "net/http" "net/http/httptest" @@ -131,7 +132,7 @@ menuentry 'LinuxKit ISO Image' { } h := &Handler{ - Logger: logr.Discard(), + Logger: logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelDebug})), Backend: &mockBackend{}, SourceISO: u, ExtraKernelParams: []string{}, @@ -149,13 +150,14 @@ menuentry 'LinuxKit ISO Image' { Method: http.MethodGet, URL: purl, } + req.Header.Set("Range", "bytes=0-") res, err := h.RoundTrip(&req) if err != nil { t.Fatal(err) } defer res.Body.Close() - if res.StatusCode != http.StatusOK { - t.Fatalf("got status code: %d, want status code: %d", res.StatusCode, http.StatusOK) + if res.StatusCode != http.StatusPartialContent { + t.Fatalf("got status code: %d, want status code: %d", res.StatusCode, http.StatusPartialContent) } isoContents, err := io.ReadAll(res.Body) From 24250a6dcccce2192de43c5db72494340ae9094f Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 21 Nov 2024 11:29:39 -0700 Subject: [PATCH 20/21] Clean up Tiltfile: Remove reference to my local machine. Signed-off-by: Jacob Weinstock --- Tiltfile | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Tiltfile b/Tiltfile index b54d1e85..3653cf45 100644 --- a/Tiltfile +++ b/Tiltfile @@ -7,11 +7,6 @@ local_resource('compile smee', deps=["go.mod", "go.sum", "internal", "Dockerfile", "cmd/smee/main.go", "cmd/smee/flag.go", "cmd/smee/backend.go"], ) -#docker_build( -# 'quay.io/tinkerbell/smee', -# '.', -# dockerfile='Dockerfile', -#) docker_build_with_restart( 'quay.io/tinkerbell/smee', '.', @@ -27,12 +22,14 @@ default_trusted_proxies = local_output("kubectl get nodes -o jsonpath='{.items[* trusted_proxies = os.getenv('TRUSTED_PROXIES', default_trusted_proxies) lb_ip = os.getenv('LB_IP', '') stack_version = os.getenv('STACK_CHART_VERSION', '0.5.0') -stack_location = os.getenv('STACK_LOCATION', '/home/tink/repos/tinkerbell/charts/tinkerbell/stack') # or a local path like '/home/tink/repos/tinkerbell/charts/tinkerbell/stack' +stack_location = os.getenv('STACK_LOCATION', 'oci://ghcr.io/tinkerbell/charts/stack') # or a local path like '/home/tink/repos/tinkerbell/charts/tinkerbell/stack' namespace = 'tink' if lb_ip == '': fail('Please set the LB_IP environment variable. This is required to deploy the stack.') +# to use a KinD cluster, add a macvlan interface into the KinD docker container. for example: `docker network connect macvlan kind-control-plane` +# Then uncomment the 2 interface lines below. helm_resource('stack', chart=stack_location, namespace=namespace, @@ -43,8 +40,8 @@ helm_resource('stack', '--version=%s' % stack_version, '--set=global.trustedProxies={%s}' % trusted_proxies, '--set=global.publicIP=%s' % lb_ip, - '--set=stack.kubevip.interface=eth1', - '--set=stack.relay.sourceInterface=eth1', + #'--set=stack.kubevip.interface=eth1', + #'--set=stack.relay.sourceInterface=eth1', ], release_name='stack' ) From edd662dea151658330522c1a9166be2ee627fa22 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 21 Nov 2024 11:36:14 -0700 Subject: [PATCH 21/21] Remove logging in ISO test: Signed-off-by: Jacob Weinstock --- internal/iso/iso_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index d35b485d..66e9f489 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "log/slog" "net" "net/http" "net/http/httptest" @@ -132,7 +131,7 @@ menuentry 'LinuxKit ISO Image' { } h := &Handler{ - Logger: logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true, Level: slog.LevelDebug})), + Logger: logr.Discard(), Backend: &mockBackend{}, SourceISO: u, ExtraKernelParams: []string{},