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

feat: cli option to enable the new action cache #1954

Merged
merged 19 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
1 change: 1 addition & 0 deletions cmd/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type Input struct {
matrix []string
actionCachePath string
logPrefixJobID bool
useNewActionCache bool
}

func (i *Input) resolve(path string) string {
Expand Down
6 changes: 6 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func Execute(ctx context.Context, version string) {
rootCmd.PersistentFlags().StringVarP(&input.cacheServerAddr, "cache-server-addr", "", common.GetOutboundIP().String(), "Defines the address to which the cache server binds.")
rootCmd.PersistentFlags().Uint16VarP(&input.cacheServerPort, "cache-server-port", "", 0, "Defines the port where the artifact server listens. 0 means a randomly available port.")
rootCmd.PersistentFlags().StringVarP(&input.actionCachePath, "action-cache-path", "", filepath.Join(CacheHomeDir, "act"), "Defines the path where the actions get cached and host workspaces created.")
rootCmd.PersistentFlags().BoolVarP(&input.useNewActionCache, "use-new-action-cache", "", false, "Enable using the new Action Cache for storing Actions locally")
rootCmd.SetArgs(args())

if err := rootCmd.Execute(); err != nil {
Expand Down Expand Up @@ -613,6 +614,11 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str
ReplaceGheActionTokenWithGithubCom: input.replaceGheActionTokenWithGithubCom,
Matrix: matrixes,
}
if input.useNewActionCache {
config.ActionCache = &runner.GoGitActionCache{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enabled behind feature cli flag: --use-new-action-cache, old code has been restored.

Path: CacheHomeDir,
ChristopherHX marked this conversation as resolved.
Show resolved Hide resolved
}
}
r, err := runner.New(config)
if err != nil {
return err
Expand Down
20 changes: 19 additions & 1 deletion pkg/container/docker_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@

archMapper := map[string]string{
"x86_64": "X64",
"386": "X86",
"aarch64": "ARM64",

Check warning on line 244 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L243-L244

Added lines #L243 - L244 were not covered by tests
}
if arch, ok := archMapper[info.Architecture]; ok {
return arch
Expand Down Expand Up @@ -345,13 +345,13 @@
return nil, nil, fmt.Errorf("Cannot parse container options: '%s': '%w'", input.Options, err)
}

if len(copts.netMode.Value()) == 0 {
if err = copts.netMode.Set("host"); err != nil {
return nil, nil, fmt.Errorf("Cannot parse networkmode=host. This is an internal error and should not happen: '%w'", err)
}

Check warning on line 351 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L348-L351

Added lines #L348 - L351 were not covered by tests
}

containerConfig, err := parse(flags, copts, runtime.GOOS)

Check warning on line 354 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L354

Added line #L354 was not covered by tests
if err != nil {
return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
}
Expand Down Expand Up @@ -586,7 +586,7 @@
}
exp := regexp.MustCompile(`\d+\n`)
found := exp.FindString(sid)
id, err := strconv.ParseInt(strings.TrimSpace(found), 10, 32)

Check warning on line 589 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L589

Added line #L589 was not covered by tests
if err != nil {
return nil
}
Expand Down Expand Up @@ -649,12 +649,30 @@
}
}

func (cr *containerReference) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error {
err := cr.cli.CopyToContainer(ctx, cr.id, destPath, tarStream, types.CopyToContainerOptions{})
// Mkdir
buf := &bytes.Buffer{}
tw := tar.NewWriter(buf)
_ = tw.WriteHeader(&tar.Header{
Name: destPath,
Mode: 777,
Typeflag: tar.TypeDir,
})
tw.Close()
err := cr.cli.CopyToContainer(ctx, cr.id, "/", buf, types.CopyToContainerOptions{})
if err != nil {
return fmt.Errorf("failed to mkdir to copy content to container: %w", err)
}

Check warning on line 665 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L652-L665

Added lines #L652 - L665 were not covered by tests
Comment on lines +674 to +686
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second CopyToContainer would fail with destPath doesn't exists.
Not using mkdir, works in containers without that tool and use plain docker api.

// Copy Content
err = cr.cli.CopyToContainer(ctx, cr.id, destPath, tarStream, types.CopyToContainerOptions{})
if err != nil {
return fmt.Errorf("failed to copy content to container: %w", err)
}

Check warning on line 670 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L667-L670

Added lines #L667 - L670 were not covered by tests
// If this fails, then folders have wrong permissions on non root container
if cr.UID != 0 || cr.GID != 0 {
_ = cr.Exec([]string{"chown", "-R", fmt.Sprintf("%d:%d", cr.UID, cr.GID), destPath}, nil, "0", "")(ctx)
}
Comment on lines +692 to +695
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As before we need to fix folder ownership

return nil

Check warning on line 675 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L672-L675

Added lines #L672 - L675 were not covered by tests
}

func (cr *containerReference) copyDir(dstPath string, srcPath string, useGitIgnore bool) common.Executor {
Expand Down
22 changes: 21 additions & 1 deletion pkg/runner/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
reader, closer, err := readFile("action.yml")
if os.IsNotExist(err) {
reader, closer, err = readFile("action.yaml")
if err != nil {
if os.IsNotExist(err) {
if _, closer, err2 := readFile("Dockerfile"); err2 == nil {
closer.Close()
action := &model.Action{
Expand Down Expand Up @@ -91,6 +91,8 @@
}
}
return nil, err
} else if err != nil {
return nil, err

Check warning on line 95 in pkg/runner/action.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/action.go#L95

Added line #L95 was not covered by tests
}
} else if err != nil {
return nil, err
Expand All @@ -110,6 +112,17 @@
if stepModel.Type() != model.StepTypeUsesActionRemote {
return nil
}

if rc.Config != nil && rc.Config.ActionCache != nil {
raction := step.(*stepActionRemote)
ta, err := rc.Config.ActionCache.GetTarArchive(ctx, raction.cacheDir, raction.resolvedSha, "")
if err != nil {
return err
}
defer ta.Close()
return rc.JobContainer.CopyTarStream(ctx, containerActionDir, ta)

Check warning on line 123 in pkg/runner/action.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/action.go#L117-L123

Added lines #L117 - L123 were not covered by tests
}

if err := removeGitIgnore(ctx, actionDir); err != nil {
return err
}
Expand Down Expand Up @@ -261,9 +274,16 @@
if localAction {
buildContext, err = rc.JobContainer.GetContainerArchive(ctx, contextDir+"/.")
if err != nil {
return err
}

Check warning on line 278 in pkg/runner/action.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/action.go#L277-L278

Added lines #L277 - L278 were not covered by tests
defer buildContext.Close()
} else if rc.Config.ActionCache != nil {
rstep := step.(*stepActionRemote)
buildContext, err = rc.Config.ActionCache.GetTarArchive(ctx, rstep.cacheDir, rstep.resolvedSha, contextDir)
if err != nil {
return err
}
defer buildContext.Close()

Check warning on line 286 in pkg/runner/action.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/action.go#L281-L286

Added lines #L281 - L286 were not covered by tests
}
prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{
ContextDir: contextDir,
Expand Down Expand Up @@ -362,8 +382,8 @@
binds, mounts := rc.GetBindsAndMounts()
networkMode := fmt.Sprintf("container:%s", rc.jobContainerName())
if rc.IsHostEnv(ctx) {
networkMode = "default"
}

Check warning on line 386 in pkg/runner/action.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/action.go#L385-L386

Added lines #L385 - L386 were not covered by tests
stepContainer := container.NewContainer(&container.NewContainerInput{
Cmd: cmd,
Entrypoint: entrypoint,
Expand Down
1 change: 1 addition & 0 deletions pkg/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub
ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub.
Matrix map[string]map[string]bool // Matrix config to run
ActionCache ActionCache
}

type caller struct {
Expand Down Expand Up @@ -94,8 +95,8 @@
}
eventJSON, err := json.Marshal(eventMap)
if err != nil {
return nil, err
}

Check warning on line 99 in pkg/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/runner.go#L98-L99

Added lines #L98 - L99 were not covered by tests
runner.eventJSON = string(eventJSON)
}
return runner, nil
Expand Down Expand Up @@ -153,7 +154,7 @@

var matrixes []map[string]interface{}
if m, err := job.GetMatrixes(); err != nil {
log.Errorf("Error while get job's matrix: %v", err)

Check warning on line 157 in pkg/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/runner.go#L157

Added line #L157 was not covered by tests
} else {
log.Debugf("Job Matrices: %v", m)
log.Debugf("Runner Matrices: %v", runner.config.Matrix)
Expand Down Expand Up @@ -185,8 +186,8 @@
executor, err := rc.Executor()

if err != nil {
return err
}

Check warning on line 190 in pkg/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/runner.go#L189-L190

Added lines #L189 - L190 were not covered by tests

return executor(common.WithJobErrorContainer(WithJobLogger(ctx, rc.Run.JobID, jobName, rc.Config, &rc.Masks, matrix)))
})
Expand All @@ -195,7 +196,7 @@
}
ncpu := runtime.NumCPU()
if 1 > ncpu {
ncpu = 1

Check warning on line 199 in pkg/runner/runner.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/runner.go#L199

Added line #L199 was not covered by tests
}
log.Debugf("Detected CPUs: %d", ncpu)
return common.NewParallelExecutor(ncpu, pipeline...)(ctx)
Expand Down
13 changes: 13 additions & 0 deletions pkg/runner/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
stepStagePost
)

// Controls how many symlinks are resolved for local and remote Actions
const maxSymlinkDepth = 10

func (s stepStage) String() string {
switch s {
case stepStagePre:
Expand All @@ -50,8 +53,8 @@
env := map[string]string{}
err := rc.JobContainer.UpdateFromEnv(path.Join(rc.JobContainer.GetActPath(), fileName), &env)(ctx)
if err != nil {
return err
}

Check warning on line 57 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L56-L57

Added lines #L56 - L57 were not covered by tests
for k, v := range env {
setter(ctx, map[string]string{"name": k}, v)
}
Expand Down Expand Up @@ -173,7 +176,7 @@
}
err = processRunnerEnvFileCommand(ctx, outputFileCommand, rc, rc.setOutput)
if err != nil {
return err

Check warning on line 179 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L179

Added line #L179 was not covered by tests
}
err = rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand))
if err != nil {
Expand All @@ -189,9 +192,9 @@
func evaluateStepTimeout(ctx context.Context, exprEval ExpressionEvaluator, stepModel *model.Step) (context.Context, context.CancelFunc) {
timeout := exprEval.Interpolate(ctx, stepModel.TimeoutMinutes)
if timeout != "" {
if timeOutMinutes, err := strconv.ParseInt(timeout, 10, 64); err == nil {
return context.WithTimeout(ctx, time.Duration(timeOutMinutes)*time.Minute)
}

Check warning on line 197 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L195-L197

Added lines #L195 - L197 were not covered by tests
}
return ctx, func() {}
}
Expand Down Expand Up @@ -274,7 +277,7 @@

func mergeIntoMap(step step, target *map[string]string, maps ...map[string]string) {
if rc := step.getRunContext(); rc != nil && rc.JobContainer != nil && rc.JobContainer.IsEnvironmentCaseInsensitive() {
mergeIntoMapCaseInsensitive(*target, maps...)

Check warning on line 280 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L280

Added line #L280 was not covered by tests
} else {
mergeIntoMapCaseSensitive(*target, maps...)
}
Expand All @@ -298,8 +301,8 @@
if k, ok := foldKeys[foldKey]; ok {
return k
}
foldKeys[strings.ToLower(foldKey)] = s
return s

Check warning on line 305 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L304-L305

Added lines #L304 - L305 were not covered by tests
}
for _, m := range maps {
for k, v := range m {
Expand All @@ -307,3 +310,13 @@
}
}
}

func symlinkJoin(filename, sym, parent string) (string, error) {
dir := path.Dir(filename)
dest := path.Join(dir, sym)
prefix := path.Clean(parent) + "/"
if strings.HasPrefix(dest, prefix) || prefix == "./" {
return dest, nil
}
return "", fmt.Errorf("symlink tries to access file '%s' outside of '%s'", strings.ReplaceAll(dest, "'", "''"), strings.ReplaceAll(parent, "'", "''"))

Check warning on line 321 in pkg/runner/step.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step.go#L314-L321

Added lines #L314 - L321 were not covered by tests
}
Comment on lines +314 to +322
Copy link
Contributor Author

@ChristopherHX ChristopherHX Jan 8, 2024

Choose a reason for hiding this comment

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

I forget that \ are technically allowed here, path wouldn't handle them...
Don't think it is a new problem

35 changes: 27 additions & 8 deletions pkg/runner/step_action_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import (
"archive/tar"
"context"
"errors"
"fmt"
"io"
"io/fs"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -42,15 +45,31 @@
localReader := func(ctx context.Context) actionYamlReader {
_, cpath := getContainerActionPaths(sal.Step, path.Join(actionDir, ""), sal.RunContext)
return func(filename string) (io.Reader, io.Closer, error) {
tars, err := sal.RunContext.JobContainer.GetContainerArchive(ctx, path.Join(cpath, filename))
if err != nil {
return nil, nil, os.ErrNotExist
spath := path.Join(cpath, filename)
for i := 0; i < maxSymlinkDepth; i++ {
tars, err := sal.RunContext.JobContainer.GetContainerArchive(ctx, spath)
if errors.Is(err, fs.ErrNotExist) {
return nil, nil, err
} else if err != nil {
return nil, nil, fs.ErrNotExist
}
treader := tar.NewReader(tars)
header, err := treader.Next()
if errors.Is(err, io.EOF) {
return nil, nil, os.ErrNotExist

Check warning on line 59 in pkg/runner/step_action_local.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_local.go#L59

Added line #L59 was not covered by tests
} else if err != nil {
return nil, nil, err
}

Check warning on line 62 in pkg/runner/step_action_local.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_local.go#L61-L62

Added lines #L61 - L62 were not covered by tests
if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink {
spath, err = symlinkJoin(spath, header.Linkname, cpath)
if err != nil {
return nil, nil, err
}

Check warning on line 67 in pkg/runner/step_action_local.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_local.go#L64-L67

Added lines #L64 - L67 were not covered by tests
} else {
return treader, tars, nil
}
}
treader := tar.NewReader(tars)
if _, err := treader.Next(); err != nil {
return nil, nil, os.ErrNotExist
}
return treader, tars, nil
return nil, nil, fmt.Errorf("max depth %d of symlinks exceeded while reading %s", maxSymlinkDepth, spath)

Check warning on line 72 in pkg/runner/step_action_local.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_local.go#L72

Added line #L72 was not covered by tests
}
}

Expand Down
43 changes: 43 additions & 0 deletions pkg/runner/step_action_remote.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package runner

import (
"archive/tar"
"context"
"errors"
"fmt"
Expand Down Expand Up @@ -28,6 +29,8 @@
action *model.Action
env map[string]string
remoteAction *remoteAction
cacheDir string
resolvedSha string
}

var (
Expand Down Expand Up @@ -60,6 +63,46 @@
github.Token = sar.RunContext.Config.ReplaceGheActionTokenWithGithubCom
}
}
if sar.RunContext.Config.ActionCache != nil {
cache := sar.RunContext.Config.ActionCache

var err error
sar.cacheDir = fmt.Sprintf("%s/%s", sar.remoteAction.Org, sar.remoteAction.Repo)
sar.resolvedSha, err = cache.Fetch(ctx, sar.cacheDir, sar.remoteAction.URL+"/"+sar.cacheDir, sar.remoteAction.Ref, github.Token)
if err != nil {
return err
}

Check warning on line 74 in pkg/runner/step_action_remote.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_remote.go#L67-L74

Added lines #L67 - L74 were not covered by tests

remoteReader := func(ctx context.Context) actionYamlReader {
return func(filename string) (io.Reader, io.Closer, error) {
spath := filename
for i := 0; i < maxSymlinkDepth; i++ {
tars, err := cache.GetTarArchive(ctx, sar.cacheDir, sar.resolvedSha, spath)
if err != nil {
return nil, nil, os.ErrNotExist
}
treader := tar.NewReader(tars)
header, err := treader.Next()
if err != nil {
return nil, nil, os.ErrNotExist
}
if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink {
spath, err = symlinkJoin(spath, header.Linkname, ".")
if err != nil {
return nil, nil, err
}
} else {
return treader, tars, nil
}

Check warning on line 96 in pkg/runner/step_action_remote.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_remote.go#L76-L96

Added lines #L76 - L96 were not covered by tests
}
return nil, nil, fmt.Errorf("max depth %d of symlinks exceeded while reading %s", maxSymlinkDepth, spath)

Check warning on line 98 in pkg/runner/step_action_remote.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_remote.go#L98

Added line #L98 was not covered by tests
}
}

actionModel, err := sar.readAction(ctx, sar.Step, sar.resolvedSha, sar.remoteAction.Path, remoteReader(ctx), os.WriteFile)
sar.action = actionModel
return err

Check warning on line 104 in pkg/runner/step_action_remote.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/step_action_remote.go#L102-L104

Added lines #L102 - L104 were not covered by tests
}

actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), safeFilename(sar.Step.Uses))
gitClone := stepActionRemoteNewCloneExecutor(git.NewGitCloneExecutorInput{
Expand Down
Loading