From ffc903e88df3231ec40aaf5b72c2917e5ad4f3c6 Mon Sep 17 00:00:00 2001 From: Matthew Baggett Date: Sat, 3 Feb 2024 22:55:41 +0100 Subject: [PATCH 1/2] Support dynamic generation of ArtifactServerPort and derived ACTIONS_RUNTIME_URL --- cmd/input.go | 2 +- cmd/root.go | 9 +++++++-- pkg/artifacts/server.go | 14 ++++++++------ pkg/artifacts/server_test.go | 8 ++++---- pkg/runner/run_context.go | 2 +- pkg/runner/runner.go | 2 +- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/cmd/input.go b/cmd/input.go index 36af6d866cd..fc13f08cd64 100644 --- a/cmd/input.go +++ b/cmd/input.go @@ -43,7 +43,7 @@ type Input struct { autoRemove bool artifactServerPath string artifactServerAddr string - artifactServerPort string + artifactServerPort uint16 noCacheServer bool cacheServerPath string cacheServerAddr string diff --git a/cmd/root.go b/cmd/root.go index 349e6ac418a..dba4e8bbee3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -90,7 +90,7 @@ func Execute(ctx context.Context, version string) { rootCmd.PersistentFlags().StringVarP(&input.githubInstance, "github-instance", "", "github.com", "GitHub instance to use. Don't use this if you are not using GitHub Enterprise Server.") rootCmd.PersistentFlags().StringVarP(&input.artifactServerPath, "artifact-server-path", "", "", "Defines the path where the artifact server stores uploads and retrieves downloads from. If not specified the artifact server will not start.") rootCmd.PersistentFlags().StringVarP(&input.artifactServerAddr, "artifact-server-addr", "", common.GetOutboundIP().String(), "Defines the address to which the artifact server binds.") - rootCmd.PersistentFlags().StringVarP(&input.artifactServerPort, "artifact-server-port", "", "34567", "Defines the port where the artifact server listens.") + rootCmd.PersistentFlags().Uint16VarP(&input.artifactServerPort, "artifact-server-port", "", 0, "Defines the port where the artifact server listens. 0 means a randomly available port.") rootCmd.PersistentFlags().BoolVarP(&input.noSkipCheckout, "no-skip-checkout", "", false, "Do not skip actions/checkout") rootCmd.PersistentFlags().BoolVarP(&input.noCacheServer, "no-cache-server", "", false, "Disable cache server") rootCmd.PersistentFlags().StringVarP(&input.cacheServerPath, "cache-server-path", "", filepath.Join(CacheHomeDir, "actcache"), "Defines the path where the cache server stores caches.") @@ -626,7 +626,7 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str return err } - cancel := artifacts.Serve(ctx, input.artifactServerPath, input.artifactServerAddr, input.artifactServerPort) + cancel, artifactServer := artifacts.Serve(ctx, input.artifactServerPath, input.artifactServerAddr, input.artifactServerPort) const cacheURLKey = "ACTIONS_CACHE_URL" var cacheHandler *artifactcache.Handler @@ -639,6 +639,11 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str envs[cacheURLKey] = cacheHandler.ExternalURL() + "/" } + const actionsRuntimeURLKey = "ACTIONS_RUNTIME_URL" + if envs[actionsRuntimeURLKey] == "" { + envs[actionsRuntimeURLKey] = fmt.Sprintf("http://%s/", artifactServer.Addr) + } + ctx = common.WithDryrun(ctx, input.dryrun) if watch, err := cmd.Flags().GetBool("watch"); err != nil { return err diff --git a/pkg/artifacts/server.go b/pkg/artifacts/server.go index 4b88ea40edd..f9c0628ef1c 100644 --- a/pkg/artifacts/server.go +++ b/pkg/artifacts/server.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "io/fs" + "net" "net/http" "os" "path/filepath" @@ -275,12 +276,12 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { }) } -func Serve(ctx context.Context, artifactPath string, addr string, port string) context.CancelFunc { +func Serve(ctx context.Context, artifactPath string, addr string, port uint16) (context.CancelFunc, *http.Server) { serverContext, cancel := context.WithCancel(ctx) logger := common.Logger(serverContext) if artifactPath == "" { - return cancel + return cancel, nil } router := httprouter.New() @@ -290,16 +291,17 @@ func Serve(ctx context.Context, artifactPath string, addr string, port string) c uploads(router, artifactPath, fsys) downloads(router, artifactPath, fsys) + listener, _ := net.Listen("tcp", fmt.Sprintf("%s:%d", addr, port)) server := &http.Server{ - Addr: fmt.Sprintf("%s:%s", addr, port), + Addr: listener.Addr().String(), ReadHeaderTimeout: 2 * time.Second, Handler: router, } // run server go func() { - logger.Infof("Start server on http://%s:%s", addr, port) - if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + logger.Debugf("Start Artifact Server on http://%s", listener.Addr()) + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { logger.Fatal(err) } }() @@ -314,5 +316,5 @@ func Serve(ctx context.Context, artifactPath string, addr string, port string) c } }() - return cancel + return cancel, server } diff --git a/pkg/artifacts/server_test.go b/pkg/artifacts/server_test.go index aeeb0598a4d..780a69cd51e 100644 --- a/pkg/artifacts/server_test.go +++ b/pkg/artifacts/server_test.go @@ -240,9 +240,9 @@ type TestJobFileInfo struct { } var ( - artifactsPath = path.Join(os.TempDir(), "test-artifacts") - artifactsAddr = "127.0.0.1" - artifactsPort = "12345" + artifactsPath = path.Join(os.TempDir(), "test-artifacts") + artifactsAddr = "127.0.0.1" + artifactsPort uint16 = 12345 ) func TestArtifactFlow(t *testing.T) { @@ -252,7 +252,7 @@ func TestArtifactFlow(t *testing.T) { ctx := context.Background() - cancel := Serve(ctx, artifactsPath, artifactsAddr, artifactsPort) + cancel, _ := Serve(ctx, artifactsPath, artifactsAddr, artifactsPort) defer cancel() platforms := map[string]string{ diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 8027f791742..f33eade83fe 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -958,7 +958,7 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon func setActionRuntimeVars(rc *RunContext, env map[string]string) { actionsRuntimeURL := os.Getenv("ACTIONS_RUNTIME_URL") if actionsRuntimeURL == "" { - actionsRuntimeURL = fmt.Sprintf("http://%s:%s/", rc.Config.ArtifactServerAddr, rc.Config.ArtifactServerPort) + actionsRuntimeURL = fmt.Sprintf("http://%s:%d/", rc.Config.ArtifactServerAddr, rc.Config.ArtifactServerPort) } env["ACTIONS_RUNTIME_URL"] = actionsRuntimeURL diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index dd8afe54819..073a37d66cc 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -53,7 +53,7 @@ type Config struct { AutoRemove bool // controls if the container is automatically removed upon workflow completion ArtifactServerPath string // the path where the artifact server stores uploads ArtifactServerAddr string // the address the artifact server binds to - ArtifactServerPort string // the port the artifact server binds to + ArtifactServerPort uint16 // the port the artifact server binds to NoSkipCheckout bool // do not skip actions/checkout RemoteName string // remote name in local git repo config ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub From 01ccdd137d98873ec56016fa3189064d5a04d167 Mon Sep 17 00:00:00 2001 From: Matthew Baggett Date: Sat, 3 Feb 2024 23:55:46 +0100 Subject: [PATCH 2/2] move duplicated, junk-injecting copy of ACTIONS_RUNTIME_URL from run_context.go. Noticed that ACTIONS_RUNTIME_TOKEN is being generated there too, all on its lonesome.. Moved to root.go to keep ACTIONS_RUNTIME_URL company... --- cmd/root.go | 8 +++++++- pkg/runner/run_context.go | 18 ------------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index dba4e8bbee3..50e6a968d8c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -640,9 +640,15 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str } const actionsRuntimeURLKey = "ACTIONS_RUNTIME_URL" - if envs[actionsRuntimeURLKey] == "" { + if !input.noCacheServer && envs[actionsRuntimeURLKey] == "" && artifactServer != nil { envs[actionsRuntimeURLKey] = fmt.Sprintf("http://%s/", artifactServer.Addr) } + const actionsRuntimeTokenKey = "ACTIONS_RUNTIME_TOKEN" + actionsRuntimeToken := os.Getenv(actionsRuntimeTokenKey) + if !input.noCacheServer && actionsRuntimeToken == "" && artifactServer != nil { + actionsRuntimeToken = "token" + } + envs[actionsRuntimeTokenKey] = actionsRuntimeToken ctx = common.WithDryrun(ctx, input.dryrun) if watch, err := cmd.Flags().GetBool("watch"); err != nil { diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index f33eade83fe..d2acd53aeb5 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -936,10 +936,6 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon env["GITHUB_API_URL"] = github.APIURL env["GITHUB_GRAPHQL_URL"] = github.GraphQLURL - if rc.Config.ArtifactServerPath != "" { - setActionRuntimeVars(rc, env) - } - for _, platformName := range rc.runsOnPlatformNames(ctx) { if platformName != "" { if platformName == "ubuntu-latest" { @@ -955,20 +951,6 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon return env } -func setActionRuntimeVars(rc *RunContext, env map[string]string) { - actionsRuntimeURL := os.Getenv("ACTIONS_RUNTIME_URL") - if actionsRuntimeURL == "" { - actionsRuntimeURL = fmt.Sprintf("http://%s:%d/", rc.Config.ArtifactServerAddr, rc.Config.ArtifactServerPort) - } - env["ACTIONS_RUNTIME_URL"] = actionsRuntimeURL - - actionsRuntimeToken := os.Getenv("ACTIONS_RUNTIME_TOKEN") - if actionsRuntimeToken == "" { - actionsRuntimeToken = "token" - } - env["ACTIONS_RUNTIME_TOKEN"] = actionsRuntimeToken -} - func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, error) { // TODO: remove below 2 lines when we can release act with breaking changes username := rc.Config.Secrets["DOCKER_USERNAME"]