Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use ETag headers for served javascript files #39251

Merged
merged 1 commit into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/httplib/httpheaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ func SetCacheHeaders(h http.Header, maxAge time.Duration) {
h.Set("Cache-Control", fmt.Sprintf("max-age=%.f, immutable", maxAge.Seconds()))
}

// SetEntityTagCacheHeaders tells proxies and browsers to cache the content
// and sets an ETag based on teleport version which can be used to check for modifications
func SetEntityTagCacheHeaders(h http.Header, etag string) {
h.Set("Cache-Control", "no-cache")
h.Set("ETag", etag)
}

// SetDefaultSecurityHeaders adds headers that should generally be considered safe defaults. It is expected that all
// responses should be able to add these headers without negative impact.
func SetDefaultSecurityHeaders(h http.Header) {
Expand Down
45 changes: 44 additions & 1 deletion lib/web/apiserver.go
Copy link
Member

Choose a reason for hiding this comment

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

Based on the thread that Steve started (#39251 (comment)), could you document why we use this specific ETag format? The thread has a lot of valuable info about correctly handling a bunch of different scenarios: multiple proxies, binaries compiled at different times for different platforms, dev version, etc.

Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,16 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {

// serve the web UI from the embedded filesystem
var indexPage *template.Template
// we will set our etag based on the teleport version and
// the webasset app hash if available. The version only will not
// suffice as it can cause incorrect caching for local development.

// The hash of the webasset app.js is used to ensure that builds at
// different times or different OSes will be the same and not cause
// cache invalidation for production users. For example, using a timestamp
// at build time would cause different OS builds to be different, and timestamps
// at process start would mean multiple proxies would serving different etags)
etag := fmt.Sprintf("W/%q", teleport.Version)
if cfg.StaticFS != nil {
index, err := cfg.StaticFS.Open("/index.html")
if err != nil {
Expand All @@ -493,6 +503,13 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {

h.Handle("GET", "/robots.txt", httplib.MakeHandler(serveRobotsTxt))
h.Handle("GET", "/web/config.js", h.WithUnauthenticatedLimiter(h.getWebConfig))

etagFromAppHash, err := readEtagFromAppHash(cfg.StaticFS)
if err != nil {
h.log.WithError(err).Error("Could not read apphash from embedded webassets. Using version only as ETag for Web UI assets.")
} else {
etag = etagFromAppHash
}
}

if cfg.NodeWatcher != nil {
Expand Down Expand Up @@ -524,10 +541,18 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {

// serve Web UI:
if strings.HasPrefix(r.URL.Path, "/web/app") {

// Check if the incoming request wants to check the version
// and if the version has not changed, return a Not Modified response
if match := r.Header.Get("If-None-Match"); match == etag {
w.WriteHeader(http.StatusNotModified)
return
}

fs := http.FileServer(cfg.StaticFS)

fs = makeGzipHandler(fs)
fs = makeCacheHandler(fs)
fs = makeCacheHandler(fs, etag)

http.StripPrefix("/web", fs).ServeHTTP(w, r)
} else if strings.HasPrefix(r.URL.Path, "/web/") || r.URL.Path == "/web" {
Expand Down Expand Up @@ -4598,3 +4623,21 @@ func serveRobotsTxt(w http.ResponseWriter, r *http.Request, p httprouter.Params)
w.Write([]byte(robots))
return nil, nil
}

func readEtagFromAppHash(fs http.FileSystem) (string, error) {
hashFile, err := fs.Open("/apphash")
if err != nil {
return "", trace.Wrap(err)
}
defer hashFile.Close()

appHash, err := io.ReadAll(hashFile)
if err != nil {
return "", trace.Wrap(err)
}

versionWithHash := fmt.Sprintf("%s-%s", teleport.Version, string(appHash))
etag := fmt.Sprintf("%q", versionWithHash)

return etag, nil
}
15 changes: 13 additions & 2 deletions lib/web/cachehandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,26 @@ package web

import (
"net/http"
"path/filepath"
"slices"
"time"

"github.com/gravitational/teleport/lib/httplib"
)

// makeCacheHandler adds support for gzip compression for given handler.
func makeCacheHandler(handler http.Handler) http.Handler {
func makeCacheHandler(handler http.Handler, etag string) http.Handler {
cachedFileTypes := []string{".woff", ".woff2", ".ttf"}
greedy52 marked this conversation as resolved.
Show resolved Hide resolved

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
httplib.SetCacheHeaders(w.Header(), time.Hour*24*365 /* one year */)
// We can cache fonts "permanently" because we don't expect them to change. The rest of our
// assets will have an ETag associated with them (teleport version) that will allow us
// to conditionally send the updated assets or a 304 status (Not Modified) response
if slices.Contains(cachedFileTypes, filepath.Ext(r.URL.Path)) {
httplib.SetCacheHeaders(w.Header(), time.Hour*24*365 /* one year */)
} else {
httplib.SetEntityTagCacheHeaders(w.Header(), etag)
}

handler.ServeHTTP(w, r)
})
Expand Down
58 changes: 58 additions & 0 deletions lib/web/cachehandler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Teleport
// Copyright (C) 2024 Gravitational, Inc.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package web

import (
"net/http"
"net/http/httptest"
"strconv"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestMakeCacheHandler(t *testing.T) {
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
})

etag := "test-etag"

recorder := httptest.NewRecorder()

req, err := http.NewRequest("GET", "/testfile.woff", nil)
if err != nil {
t.Fatal(err)
}

cacheHandler := makeCacheHandler(testHandler, etag)

cacheHandler.ServeHTTP(recorder, req)

expectedCacheControl := "max-age=" + strconv.Itoa(int(time.Hour*24*365/time.Second)) + ", immutable"
require.Equal(t, expectedCacheControl, recorder.Header().Get("Cache-Control"))

req2, err := http.NewRequest("GET", "/testfile.css", nil)
if err != nil {
t.Fatal(err)
}

cacheHandler.ServeHTTP(recorder, req2)

require.Equal(t, etag, recorder.Header().Get("ETag"))
}
43 changes: 43 additions & 0 deletions web/packages/build/vite/apphash.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Teleport
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { createHash } from 'crypto';
import { writeFileSync } from 'fs';
import { resolve } from 'path';

// this plugin is used to generate a file containing the hash of the app.js
// bundle. Because we omit the hash from the filename, we don't have access to it
// in the build chain. We generate a hash using the same methods used when files
// passed by default to `augmentChunkHash` (hash.update).
// https://rollupjs.org/plugin-development/#augmentchunkhash
export function generateAppHashFile(outputDir: string, entryFilename: string) {
return {
name: 'app-hash-plugin',
generateBundle(_, bundle) {
// bundle is OutputChunk | OutputAsset. These types aren't exported
// by vite but by rollup, which isn't directly in our bundle so we
// will use `any` instead of installing rollup
// https://rollupjs.org/plugin-development/#generatebundle
const { code } = bundle[entryFilename] as any;
if (code) {
const hash = createHash('sha256').update(code).digest('base64');
writeFileSync(resolve(outputDir, 'apphash'), hash);
}
},
};
}
7 changes: 5 additions & 2 deletions web/packages/build/vite/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import wasm from 'vite-plugin-wasm';

import { htmlPlugin, transformPlugin } from './html';
import { getStyledComponentsConfig } from './styled';
import { generateAppHashFile } from './apphash';

import type { UserConfig } from 'vite';

const DEFAULT_PROXY_TARGET = '127.0.0.1:3080';
const ENTRY_FILE_NAME = 'app/app.js';

export function createViteConfig(
rootDirectory: string,
Expand Down Expand Up @@ -71,8 +73,8 @@ export function createViteConfig(
emptyOutDir: true,
rollupOptions: {
output: {
// removes hashing from our entry point file
entryFileNames: 'app/app.js',
// removes hashing from our entry point file.
entryFileNames: ENTRY_FILE_NAME,
// assist is still lazy loaded and the telemetry bundle breaks any
// websocket connections if included in the bundle. We will leave these two
// files out of the bundle but without hashing so they are still discoverable.
Expand Down Expand Up @@ -100,6 +102,7 @@ export function createViteConfig(
projects: [resolve(rootDirectory, 'tsconfig.json')],
}),
transformPlugin(),
generateAppHashFile(outputDirectory, ENTRY_FILE_NAME),
wasm(),
],
define: {
Expand Down
Loading