From 10aaf3eb1873d88d3f9ab3027d9452b40c91d9d9 Mon Sep 17 00:00:00 2001 From: Son Bui Date: Fri, 6 Dec 2024 21:00:19 +0700 Subject: [PATCH] fix: replace deprecated `bouk/staticfiles` with native Go `embed`. fixes #11654 (#11707) Signed-off-by: Son Bui --- .github/workflows/ci-build.yaml | 2 +- .golangci.yml | 2 - Makefile | 33 ++++------- dev/nix/conf.nix | 1 - dev/nix/flake.nix | 13 ---- server/apiserver/argoserver.go | 3 +- server/static/files.go | 8 --- server/static/files.go.stub | 8 --- server/static/response-rewriter.go | 20 ------- server/static/static.go | 72 +++++++++++++++++----- server/static/static_test.go | 95 ++++++++++++++++++++++++++++++ ui/embed.go | 10 ++++ 12 files changed, 177 insertions(+), 90 deletions(-) delete mode 100644 server/static/files.go delete mode 100644 server/static/files.go.stub delete mode 100644 server/static/response-rewriter.go create mode 100644 server/static/static_test.go create mode 100644 ui/embed.go diff --git a/.github/workflows/ci-build.yaml b/.github/workflows/ci-build.yaml index a3fb946a11bd..e82fc021be95 100644 --- a/.github/workflows/ci-build.yaml +++ b/.github/workflows/ci-build.yaml @@ -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 diff --git a/.golangci.yml b/.golangci.yml index b8b0227bd4f6..5574b2e7ff18 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,5 +65,3 @@ issues: - sdks - ui - vendor - exclude-files: - - server/static/files.go diff --git a/Makefile b/Makefile index 365bbe42df3e..c51b9e3d77e7 100644 --- a/Makefile +++ b/Makefile @@ -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 @@ -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 @@ -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 @@ -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`. @@ -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 @@ -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 diff --git a/dev/nix/conf.nix b/dev/nix/conf.nix index 1850eb2d0f22..520f6b187434 100644 --- a/dev/nix/conf.nix +++ b/dev/nix/conf.nix @@ -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"; diff --git a/dev/nix/flake.nix b/dev/nix/flake.nix index 9152352e25ec..8e02eebddd71 100644 --- a/dev/nix/flake.nix +++ b/dev/nix/flake.nix @@ -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}; }; @@ -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 @@ -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 diff --git a/server/apiserver/argoserver.go b/server/apiserver/argoserver.go index 390aaa3b371b..1369c384307a 100644 --- a/server/apiserver/argoserver.go +++ b/server/apiserver/argoserver.go @@ -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" @@ -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 } diff --git a/server/static/files.go b/server/static/files.go deleted file mode 100644 index 7179551ae2c9..000000000000 --- a/server/static/files.go +++ /dev/null @@ -1,8 +0,0 @@ -// File built without static files -package static - -import "net/http" - -func ServeHTTP(http.ResponseWriter, *http.Request) {} - -func Hash(string) string { return "" } diff --git a/server/static/files.go.stub b/server/static/files.go.stub deleted file mode 100644 index 7179551ae2c9..000000000000 --- a/server/static/files.go.stub +++ /dev/null @@ -1,8 +0,0 @@ -// File built without static files -package static - -import "net/http" - -func ServeHTTP(http.ResponseWriter, *http.Request) {} - -func Hash(string) string { return "" } diff --git a/server/static/response-rewriter.go b/server/static/response-rewriter.go deleted file mode 100644 index b824143ff7a7..000000000000 --- a/server/static/response-rewriter.go +++ /dev/null @@ -1,20 +0,0 @@ -package static - -import ( - "bytes" - "net/http" - "strconv" -) - -type responseRewriter struct { - http.ResponseWriter - old []byte - new []byte -} - -func (w *responseRewriter) Write(a []byte) (int, error) { - b := bytes.Replace(a, w.old, w.new, 1) - // status code and headers are printed out when we write data - w.Header().Set("Content-Length", strconv.Itoa(len(b))) - return w.ResponseWriter.Write(b) -} diff --git a/server/static/static.go b/server/static/static.go index f2df6d60b6f2..f9b03752a4d0 100644 --- a/server/static/static.go +++ b/server/static/static.go @@ -1,9 +1,17 @@ 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 { @@ -11,23 +19,16 @@ type FilesServer struct { 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(``) + +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(``), new: []byte(fmt.Sprintf(``, s.baseHRef))} - } if s.xframeOpts != "" { w.Header().Set("X-Frame-Options", s.xframeOpts) @@ -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(``, 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) } diff --git a/server/static/static_test.go b/server/static/static_test.go new file mode 100644 index 000000000000..e9682dbfe2c3 --- /dev/null +++ b/server/static/static_test.go @@ -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: ` + + + + Argo + + + + + + + + +
+ +`, + expected: ` + + + + Argo + + + + + + + + +
+ +`, + replaceWith: ``, + }, + { + name: "root basepath", + data: ` + + + + Argo + + + + + + + + +
+ +`, + expected: ` + + + + Argo + + + + + + + + +
+ +`, + replaceWith: ``, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := replaceBaseHRef(testCase.data, testCase.replaceWith) + assert.Equal(t, testCase.expected, result) + }) + } +} diff --git a/ui/embed.go b/ui/embed.go new file mode 100644 index 000000000000..5c9792fa48d7 --- /dev/null +++ b/ui/embed.go @@ -0,0 +1,10 @@ +package ui + +import "embed" + +const EMBED_PATH = "dist/app" + +// Embedded contains embedded UI resources +// +//go:embed dist/app +var Embedded embed.FS