Skip to content

Commit

Permalink
fix: replace deprecated bouk/staticfiles with native Go embed. fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sonbui00 authored Dec 6, 2024
1 parent 9d1d2cd commit 10aaf3e
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 90 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ jobs:
go-version: "1.23"
cache: true
# windows run does not use makefile target because it does a lot more than just testing and is not cross-platform compatible
- run: go test -p 20 -covermode=atomic -coverprofile='coverage.out' $(go list ./... | select-string -Pattern 'github.com/argoproj/argo-workflows/v3/workflow/controller' , 'github.com/argoproj/argo-workflows/v3/server' -NotMatch)
- run: if (!(Test-Path "ui/dist/app/index.html")) { New-Item -ItemType Directory -Force -Path "ui/dist/app" | Out-Null; New-Item -ItemType File -Path "ui/dist/app/placeholder" | Out-Null }; go test -p 20 -covermode=atomic -coverprofile='coverage.out' $(go list ./... | select-string -Pattern 'github.com/argoproj/argo-workflows/v3/workflow/controller' , 'github.com/argoproj/argo-workflows/v3/server' -NotMatch)
env:
KUBECONFIG: /dev/null
- name: Upload coverage report
Expand Down
2 changes: 0 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,3 @@ issues:
- sdks
- ui
- vendor
exclude-files:
- server/static/files.go
33 changes: 11 additions & 22 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ endif
HACK_PKG_FILES_AS_PKGS ?= false
ifeq ($(HACK_PKG_FILES_AS_PKGS),false)
ARGOEXEC_PKG_FILES := $(shell go list -f '{{ join .Deps "\n" }}' ./cmd/argoexec/ | grep 'argoproj/argo-workflows/v3/' | xargs go list -f '{{ range $$file := .GoFiles }}{{ print $$.ImportPath "/" $$file "\n" }}{{ end }}' | cut -c 39-)
CLI_PKG_FILES := $(shell go list -f '{{ join .Deps "\n" }}' ./cmd/argo/ | grep 'argoproj/argo-workflows/v3/' | xargs go list -f '{{ range $$file := .GoFiles }}{{ print $$.ImportPath "/" $$file "\n" }}{{ end }}' | cut -c 39-)
CLI_PKG_FILES := $(shell [ -f ui/dist/app/index.html ] || (mkdir -p ui/dist/app && touch ui/dist/app/placeholder); go list -f '{{ join .Deps "\n" }}' ./cmd/argo/ | grep 'argoproj/argo-workflows/v3/' | xargs go list -f '{{ range $$file := .GoFiles }}{{ print $$.ImportPath "/" $$file "\n" }}{{ end }}' | cut -c 39-)
CONTROLLER_PKG_FILES := $(shell go list -f '{{ join .Deps "\n" }}' ./cmd/workflow-controller/ | grep 'argoproj/argo-workflows/v3/' | xargs go list -f '{{ range $$file := .GoFiles }}{{ print $$.ImportPath "/" $$file "\n" }}{{ end }}' | cut -c 39-)
else
# Building argoexec on windows cannot rebuild the openapi, we need to fall back to the old
Expand Down Expand Up @@ -165,25 +165,14 @@ endef
cli: dist/argo

ui/dist/app/index.html: $(shell find ui/src -type f && find ui -maxdepth 1 -type f)
ifeq ($(STATIC_FILES),true)
# `yarn install` is fast (~2s), so you can call it safely.
JOBS=max yarn --cwd ui install
# `yarn build` is slow, so we guard it with a up-to-date check.
JOBS=max yarn --cwd ui build

$(GOPATH)/bin/staticfiles: Makefile
# update this in Nix when updating it here
ifneq ($(USE_NIX), true)
go install bou.ke/staticfiles@dd04075
endif

ifeq ($(STATIC_FILES),true)
server/static/files.go: $(GOPATH)/bin/staticfiles ui/dist/app/index.html
# Pack UI into a Go file
$(GOPATH)/bin/staticfiles -o server/static/files.go ui/dist/app
else
server/static/files.go:
# Building without static files
cp ./server/static/files.go.stub ./server/static/files.go
@mkdir -p ui/dist/app
touch ui/dist/app/index.html
endif

dist/argo-linux-amd64: GOARGS = GOOS=linux GOARCH=amd64
Expand All @@ -198,16 +187,16 @@ dist/argo-windows-amd64: GOARGS = GOOS=windows GOARCH=amd64
dist/argo-windows-%.gz: dist/argo-windows-%
gzip --force --keep dist/argo-windows-$*.exe

dist/argo-windows-%: server/static/files.go $(CLI_PKG_FILES) go.sum
dist/argo-windows-%: ui/dist/app/index.html $(CLI_PKG_FILES) go.sum
CGO_ENABLED=0 $(GOARGS) go build -v -gcflags '${GCFLAGS}' -ldflags '${LDFLAGS} -extldflags -static' -o $@.exe ./cmd/argo

dist/argo-%.gz: dist/argo-%
gzip --force --keep dist/argo-$*

dist/argo-%: server/static/files.go $(CLI_PKG_FILES) go.sum
dist/argo-%: ui/dist/app/index.html $(CLI_PKG_FILES) go.sum
CGO_ENABLED=0 $(GOARGS) go build -v -gcflags '${GCFLAGS}' -ldflags '${LDFLAGS} -extldflags -static' -o $@ ./cmd/argo

dist/argo: server/static/files.go $(CLI_PKG_FILES) go.sum
dist/argo: ui/dist/app/index.html $(CLI_PKG_FILES) go.sum
ifeq ($(shell uname -s),Darwin)
# if local, then build fast: use CGO and dynamic-linking
go build -v -gcflags '${GCFLAGS}' -ldflags '${LDFLAGS}' -o $@ ./cmd/argo
Expand Down Expand Up @@ -454,7 +443,7 @@ $(GOPATH)/bin/golangci-lint: Makefile
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b `go env GOPATH`/bin v1.61.0

.PHONY: lint
lint: server/static/files.go $(GOPATH)/bin/golangci-lint
lint: ui/dist/app/index.html $(GOPATH)/bin/golangci-lint
rm -Rf v3 vendor
# If you're using `woc.wf.Spec` or `woc.execWf.Status` your code probably won't work with WorkflowTemplate.
# * Change `woc.wf.Spec` to `woc.execWf.Spec`.
Expand All @@ -471,7 +460,7 @@ lint: server/static/files.go $(GOPATH)/bin/golangci-lint

# for local we have a faster target that prints to stdout, does not use json, and can cache because it has no coverage
.PHONY: test
test: server/static/files.go
test: ui/dist/app/index.html
go build ./...
env KUBECONFIG=/dev/null $(GOTEST) ./...
# marker file, based on it's modification time, we know how long ago this target was run
Expand Down Expand Up @@ -702,11 +691,11 @@ go-diagrams/diagram.dot: ./hack/docs/diagram.go
docs/assets/diagram.png: go-diagrams/diagram.dot
cd go-diagrams && dot -Tpng diagram.dot -o ../docs/assets/diagram.png

docs/fields.md: api/openapi-spec/swagger.json $(shell find examples -type f) hack/docs/fields.go
docs/fields.md: api/openapi-spec/swagger.json $(shell find examples -type f) ui/dist/app/index.html hack/docs/fields.go
env ARGO_SECURE=false ARGO_INSECURE_SKIP_VERIFY=false ARGO_SERVER= ARGO_INSTANCEID= go run ./hack/docs fields

# generates several other files
docs/cli/argo.md: $(CLI_PKG_FILES) go.sum server/static/files.go hack/docs/cli.go
docs/cli/argo.md: $(CLI_PKG_FILES) go.sum ui/dist/app/index.html hack/docs/cli.go
go run ./hack/docs cli

# docs
Expand Down
1 change: 0 additions & 1 deletion dev/nix/conf.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# Even then the buildFlags are not passed into Go, meaning you won't see the correct version info yet.
# This is only intended for quick developing at the moment, gradually more functionality will be pushed here.
rec {
staticFiles = false; # not acted upon
version = "latest";
env = {
DEFAULT_REQUEUE_TIME = "1s";
Expand Down
13 changes: 0 additions & 13 deletions dev/nix/flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,6 @@
doCheck = false;
};

staticfiles = pkgs.buildGoPackage rec {
name = "staticfiles";
src = pkgs.fetchFromGitHub {
owner = "bouk";
repo = "staticfiles";
rev = "827d7f6389cd410d0aa3f3d472a4838557bf53dd";
sha256 = "0xarhmsqypl8036w96ssdzjv3k098p2d4mkmw5f6hkp1m3j67j61";
};

goPackagePath = "bou.ke/staticfiles";
};
default = config.packages.${package.name};
};

Expand All @@ -338,7 +327,6 @@
config.packages.k8sio-tools
config.packages.goreman
config.packages.stern
config.packages.staticfiles
config.packages.${package.name}
nodePackages.shell.nodeDependencies
gopls
Expand Down Expand Up @@ -368,7 +356,6 @@
config.packages.k8sio-tools
config.packages.goreman
config.packages.stern
config.packages.staticfiles
config.packages.${package.name}
nodePackages.shell.nodeDependencies
gopls
Expand Down
3 changes: 2 additions & 1 deletion server/apiserver/argoserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
"github.com/argoproj/argo-workflows/v3/server/workflow/store"
"github.com/argoproj/argo-workflows/v3/server/workflowarchive"
"github.com/argoproj/argo-workflows/v3/server/workflowtemplate"
"github.com/argoproj/argo-workflows/v3/ui"
grpcutil "github.com/argoproj/argo-workflows/v3/util/grpc"
"github.com/argoproj/argo-workflows/v3/util/instanceid"
"github.com/argoproj/argo-workflows/v3/util/json"
Expand Down Expand Up @@ -435,7 +436,7 @@ func (as *argoServer) newHTTPServer(ctx context.Context, port int, artifactServe

})
// we only enable HTST if we are secure mode, otherwise you would never be able access the UI
mux.HandleFunc("/", static.NewFilesServer(as.baseHRef, as.tlsConfig != nil && as.hsts, as.xframeOptions, as.accessControlAllowOrigin).ServerFiles)
mux.HandleFunc("/", static.NewFilesServer(as.baseHRef, as.tlsConfig != nil && as.hsts, as.xframeOptions, as.accessControlAllowOrigin, ui.Embedded).ServerFiles)
return &httpServer
}

Expand Down
8 changes: 0 additions & 8 deletions server/static/files.go

This file was deleted.

8 changes: 0 additions & 8 deletions server/static/files.go.stub

This file was deleted.

20 changes: 0 additions & 20 deletions server/static/response-rewriter.go

This file was deleted.

72 changes: 58 additions & 14 deletions server/static/static.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
package static

import (
"bytes"
"embed"
"fmt"
"io/fs"
"net/http"
"regexp"
"strings"
"time"

"github.com/argoproj/argo-workflows/v3"
"github.com/argoproj/argo-workflows/v3/ui"
)

type FilesServer struct {
baseHRef string
hsts bool
xframeOpts string
corsAllowOrigin string
staticAssets embed.FS
}

func NewFilesServer(baseHRef string, hsts bool, xframeOpts string, corsAllowOrigin string) *FilesServer {
return &FilesServer{baseHRef, hsts, xframeOpts, corsAllowOrigin}
var baseHRefRegex = regexp.MustCompile(`<base href="(.*?)">`)

func NewFilesServer(baseHRef string, hsts bool, xframeOpts string, corsAllowOrigin string, staticAssets embed.FS) *FilesServer {
return &FilesServer{baseHRef, hsts, xframeOpts, corsAllowOrigin, staticAssets}
}

func (s *FilesServer) ServerFiles(w http.ResponseWriter, r *http.Request) {
// If there is no stored static file, we'll redirect to the js app
if Hash(strings.TrimLeft(r.URL.Path, "/")) == "" {
r.URL.Path = "index.html"
}

if r.URL.Path == "index.html" {
// hack to prevent ServerHTTP from giving us gzipped content which we can do our search-and-replace on
r.Header.Del("Accept-Encoding")
w = &responseRewriter{ResponseWriter: w, old: []byte(`<base href="/">`), new: []byte(fmt.Sprintf(`<base href="%s">`, s.baseHRef))}
}

if s.xframeOpts != "" {
w.Header().Set("X-Frame-Options", s.xframeOpts)
Expand All @@ -50,6 +51,49 @@ func (s *FilesServer) ServerFiles(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Strict-Transport-Security", "max-age=31536000")
}

// in my IDE (IntelliJ) the next line is red for some reason - but this is fine
ServeHTTP(w, r)
if r.URL.Path == "/" || !s.uiAssetExists(r.URL.Path) {
data, err := s.getIndexData()
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}

modTime, err := time.Parse(argo.GetVersion().BuildDate, time.RFC3339)
if err != nil {
modTime = time.Now()
}
http.ServeContent(w, r, "index.html", modTime, bytes.NewReader(data))
} else {
staticFS, _ := fs.Sub(s.staticAssets, ui.EMBED_PATH)
http.FileServer(http.FS(staticFS)).ServeHTTP(w, r)
}
}

func (s *FilesServer) getIndexData() ([]byte, error) {
data, err := s.staticAssets.ReadFile(ui.EMBED_PATH + "/index.html")
if err != nil {
return data, err
}
if s.baseHRef != "/" && s.baseHRef != "" {
data = []byte(replaceBaseHRef(string(data), fmt.Sprintf(`<base href="/%s/">`, strings.Trim(s.baseHRef, "/"))))
}

return data, nil
}

func (s *FilesServer) uiAssetExists(filename string) bool {
f, err := s.staticAssets.Open(ui.EMBED_PATH + filename)
if err != nil {
return false
}
defer f.Close()
stat, err := f.Stat()
if err != nil {
return false
}
return !stat.IsDir()
}

func replaceBaseHRef(data string, replaceWith string) string {
return baseHRefRegex.ReplaceAllString(data, replaceWith)
}
95 changes: 95 additions & 0 deletions server/static/static_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package static

import (
"testing"

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

func TestReplaceBaseHRef(t *testing.T) {
testCases := []struct {
name string
data string
expected string
replaceWith string
}{
{
name: "non-root basepath",
data: `<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Argo</title>
<base href="/">
<meta name="viewport" content="width=device-width,initial-scale=1">
<meta name="robots" content="noindex">
<link rel="icon" type="image/png" href="assets/favicon/favicon-32x32.png" sizes="32x32">
<link rel="icon" type="image/png" href="assets/favicon/favicon-16x16.png" sizes="16x16">
<script defer="defer" src="main.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>`,
expected: `<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Argo</title>
<base href="/path1/path2/path3/">
<meta name="viewport" content="width=device-width,initial-scale=1">
<meta name="robots" content="noindex">
<link rel="icon" type="image/png" href="assets/favicon/favicon-32x32.png" sizes="32x32">
<link rel="icon" type="image/png" href="assets/favicon/favicon-16x16.png" sizes="16x16">
<script defer="defer" src="main.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>`,
replaceWith: `<base href="/path1/path2/path3/">`,
},
{
name: "root basepath",
data: `<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Argo</title>
<base href="/any/path/test/">
<meta name="viewport" content="width=device-width,initial-scale=1">
<meta name="robots" content="noindex">
<link rel="icon" type="image/png" href="assets/favicon/favicon-32x32.png" sizes="32x32">
<link rel="icon" type="image/png" href="assets/favicon/favicon-16x16.png" sizes="16x16">
<script defer="defer" src="main.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>`,
expected: `<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Argo</title>
<base href="/">
<meta name="viewport" content="width=device-width,initial-scale=1">
<meta name="robots" content="noindex">
<link rel="icon" type="image/png" href="assets/favicon/favicon-32x32.png" sizes="32x32">
<link rel="icon" type="image/png" href="assets/favicon/favicon-16x16.png" sizes="16x16">
<script defer="defer" src="main.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>`,
replaceWith: `<base href="/">`,
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
result := replaceBaseHRef(testCase.data, testCase.replaceWith)
assert.Equal(t, testCase.expected, result)
})
}
}
Loading

0 comments on commit 10aaf3e

Please sign in to comment.