From d28ad0ed41ed96d3207901b8b0d03d73ea99478e Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Sat, 19 Oct 2024 17:36:35 -0400 Subject: [PATCH 1/4] Remove else statements from if/return blocks. Also, sort runner.Config, model.Githubcontext fields to be clearer. --- pkg/common/git/git.go | 20 ++++++++++---- pkg/runner/run_context.go | 32 +++++++++++----------- pkg/runner/runner.go | 56 +++++++++++++++++++-------------------- 3 files changed, 59 insertions(+), 49 deletions(-) diff --git a/pkg/common/git/git.go b/pkg/common/git/git.go index 706f9ca5802..1afeac3325f 100644 --- a/pkg/common/git/git.go +++ b/pkg/common/git/git.go @@ -201,18 +201,28 @@ func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error func findGitSlug(url string, githubInstance string) (string, string, error) { if matches := codeCommitHTTPRegex.FindStringSubmatch(url); matches != nil { return "CodeCommit", matches[2], nil - } else if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil { + } + + if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil { return "CodeCommit", matches[2], nil - } else if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil { + } + + if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil { return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil - } else if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil { + } + + if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil { return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil - } else if githubInstance != "github.com" { + } + + if githubInstance != "github.com" { gheHTTPRegex := regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance)) gheSSHRegex := regexp.MustCompile(fmt.Sprintf(`%s[:/](.+)/(.+?)(?:.git)?$`, githubInstance)) if matches := gheHTTPRegex.FindStringSubmatch(url); matches != nil { return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil - } else if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil { + } + + if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil { return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil } } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index b8f338eef96..8712b6c89ae 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -856,30 +856,30 @@ func (rc *RunContext) getStepsContext() map[string]*model.StepResult { func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext { logger := common.Logger(ctx) ghc := &model.GithubContext{ - Event: make(map[string]interface{}), - Workflow: rc.Run.Workflow.Name, - RunAttempt: rc.Config.Env["GITHUB_RUN_ATTEMPT"], - RunID: rc.Config.Env["GITHUB_RUN_ID"], - RunNumber: rc.Config.Env["GITHUB_RUN_NUMBER"], - Actor: rc.Config.Actor, - EventName: rc.Config.EventName, Action: rc.CurrentStep, - Token: rc.Config.Token, - Job: rc.Run.JobID, ActionPath: rc.ActionPath, - ActionRepository: rc.Env["GITHUB_ACTION_REPOSITORY"], ActionRef: rc.Env["GITHUB_ACTION_REF"], + ActionRepository: rc.Env["GITHUB_ACTION_REPOSITORY"], + Actor: rc.Config.Actor, + BaseRef: rc.Config.Env["GITHUB_BASE_REF"], + Event: make(map[string]interface{}), + EventName: rc.Config.EventName, + HeadRef: rc.Config.Env["GITHUB_HEAD_REF"], + Job: rc.Run.JobID, + Ref: rc.Config.Env["GITHUB_REF"], + RefName: rc.Config.Env["GITHUB_REF_NAME"], + RefType: rc.Config.Env["GITHUB_REF_TYPE"], + Repository: rc.Config.Env["GITHUB_REPOSITORY"], RepositoryOwner: rc.Config.Env["GITHUB_REPOSITORY_OWNER"], RetentionDays: rc.Config.Env["GITHUB_RETENTION_DAYS"], + RunAttempt: rc.Config.Env["GITHUB_RUN_ATTEMPT"], + RunID: rc.Config.Env["GITHUB_RUN_ID"], + RunNumber: rc.Config.Env["GITHUB_RUN_NUMBER"], RunnerPerflog: rc.Config.Env["RUNNER_PERFLOG"], RunnerTrackingID: rc.Config.Env["RUNNER_TRACKING_ID"], - Repository: rc.Config.Env["GITHUB_REPOSITORY"], - Ref: rc.Config.Env["GITHUB_REF"], Sha: rc.Config.Env["SHA_REF"], - RefName: rc.Config.Env["GITHUB_REF_NAME"], - RefType: rc.Config.Env["GITHUB_REF_TYPE"], - BaseRef: rc.Config.Env["GITHUB_BASE_REF"], - HeadRef: rc.Config.Env["GITHUB_HEAD_REF"], + Token: rc.Config.Token, + Workflow: rc.Run.Workflow.Name, Workspace: rc.Config.Env["GITHUB_WORKSPACE"], } if rc.JobContainer != nil { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index dd8afe54819..1755d595166 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -20,47 +20,47 @@ type Runner interface { // Config contains the config for a new runner type Config struct { - Actor string // the user that triggered the event - Workdir string // path to working directory + ActionCache ActionCache // Use a custom ActionCache Implementation ActionCacheDir string // path used for caching action contents ActionOfflineMode bool // when offline, use caching action contents + Actor string // the user that triggered the event + ArtifactServerAddr string // the address the artifact server binds to + ArtifactServerPath string // the path where the artifact server stores uploads + ArtifactServerPort string // the port the artifact server binds to + AutoRemove bool // controls if the container is automatically removed upon workflow completion BindWorkdir bool // bind the workdir to the job container + ContainerArchitecture string // Desired OS/architecture platform for running containers + ContainerCapAdd []string // list of kernel capabilities to add to the containers + ContainerCapDrop []string // list of kernel capabilities to remove from the containers + ContainerDaemonSocket string // Path to Docker daemon socket + ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) + ContainerOptions string // Options for the job container + DefaultBranch string // name of the main branch for this repository + Env map[string]string // env for containers EventName string // name of event to run EventPath string // path to JSON file to use for event.json in containers - DefaultBranch string // name of the main branch for this repository - ReuseContainers bool // reuse containers to maintain state ForcePull bool // force pulling of the image, even if already present ForceRebuild bool // force rebuilding local docker image action - LogOutput bool // log the output from docker run - JSONLogger bool // use json or text logger - LogPrefixJobID bool // switches from the full job name to the job id - Env map[string]string // env for containers + GitHubInstance string // GitHub instance to use, default "github.com" Inputs map[string]string // manually passed action inputs - Secrets map[string]string // list of secrets - Vars map[string]string // list of vars - Token string // GitHub token InsecureSecrets bool // switch hiding output when printing to terminal + JSONLogger bool // use json or text logger + LogOutput bool // log the output from docker run + LogPrefixJobID bool // switches from the full job name to the job id + Matrix map[string]map[string]bool // Matrix config to run + NoSkipCheckout bool // do not skip actions/checkout Platforms map[string]string // list of platforms Privileged bool // use privileged mode - UsernsMode string // user namespace to use - ContainerArchitecture string // Desired OS/architecture platform for running containers - ContainerDaemonSocket string // Path to Docker daemon socket - ContainerOptions string // Options for the job container - UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true - GitHubInstance string // GitHub instance to use, default "github.com" - ContainerCapAdd []string // list of kernel capabilities to add to the containers - ContainerCapDrop []string // list of kernel capabilities to remove from the containers - 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 - 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 ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. - Matrix map[string]map[string]bool // Matrix config to run - ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) - ActionCache ActionCache // Use a custom ActionCache Implementation + ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub + ReuseContainers bool // reuse containers to maintain state + Secrets map[string]string // list of secrets + Token string // GitHub token + UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true + UsernsMode string // user namespace to use + Vars map[string]string // list of vars + Workdir string // path to working directory } type caller struct { From 618e211bdc8cd979a9a9a610f96d2223a410798e Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Sat, 19 Oct 2024 17:47:24 -0400 Subject: [PATCH 2/4] More if/return cleanup. --- pkg/runner/run_context.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 8712b6c89ae..efa64a55122 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -990,13 +990,18 @@ func nestedMapLookup(m map[string]interface{}, ks ...string) (rval interface{}) } if rval, ok = m[ks[0]]; !ok { return nil - } else if len(ks) == 1 { // we've reached the final key + } + + if len(ks) == 1 { // we've reached the final key return rval - } else if m, ok = rval.(map[string]interface{}); !ok { + } + + if m, ok = rval.(map[string]interface{}); !ok { return nil - } else { // 1+ more keys - return nestedMapLookup(m, ks[1:]...) } + + // 1+ more keys + return nestedMapLookup(m, ks[1:]...) } func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) map[string]string { From 644b16e365ef02cdd366af1f122e22f8b27c05d5 Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Wed, 30 Oct 2024 12:20:46 -0400 Subject: [PATCH 3/4] Simplify findGitSlug. --- pkg/common/git/git.go | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/pkg/common/git/git.go b/pkg/common/git/git.go index 1afeac3325f..911ef3c1578 100644 --- a/pkg/common/git/git.go +++ b/pkg/common/git/git.go @@ -198,31 +198,34 @@ func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error return remote.Config().URLs[0], nil } -func findGitSlug(url string, githubInstance string) (string, string, error) { - if matches := codeCommitHTTPRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2], nil - } +type findStringSubmatcher interface { + FindStringSubmatch(string) []string +} - if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2], nil +func matchesRegex(url string, matchers ...findStringSubmatcher) []string { + for _, regex := range matchers { + if matches := regex.FindStringSubmatch(url); matches != nil { + return matches + } } - if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil + return nil +} + +func findGitSlug(url string, githubInstance string) (string, string, error) { + if matches := matchesRegex(url, codeCommitHTTPRegex, codeCommitSSHRegex); matches != nil { + return "CodeCommit", matches[2], nil } - if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil { + if matches := matchesRegex(url, githubHTTPRegex, githubSSHRegex); matches != nil { return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil } if githubInstance != "github.com" { - gheHTTPRegex := regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance)) - gheSSHRegex := regexp.MustCompile(fmt.Sprintf(`%s[:/](.+)/(.+?)(?:.git)?$`, githubInstance)) - if matches := gheHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil - } - - if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil { + if matches := matchesRegex(url, + regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance)), + regexp.MustCompile(fmt.Sprintf(`%s[:/](.+)/(.+?)(?:.git)?$`, githubInstance)), + ); matches != nil { return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil } } From b1e6d3b577337dc3e8f6f04a62d54176eafee20a Mon Sep 17 00:00:00 2001 From: Steven Edwards Date: Wed, 30 Oct 2024 12:47:52 -0400 Subject: [PATCH 4/4] Revert "Remove else statements from if/return blocks." This reverts commit d28ad0ed41ed96d3207901b8b0d03d73ea99478e. --- pkg/runner/run_context.go | 32 +++++++++++----------- pkg/runner/runner.go | 56 +++++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index efa64a55122..a412aebc181 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -856,30 +856,30 @@ func (rc *RunContext) getStepsContext() map[string]*model.StepResult { func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext { logger := common.Logger(ctx) ghc := &model.GithubContext{ - Action: rc.CurrentStep, - ActionPath: rc.ActionPath, - ActionRef: rc.Env["GITHUB_ACTION_REF"], - ActionRepository: rc.Env["GITHUB_ACTION_REPOSITORY"], - Actor: rc.Config.Actor, - BaseRef: rc.Config.Env["GITHUB_BASE_REF"], Event: make(map[string]interface{}), + Workflow: rc.Run.Workflow.Name, + RunAttempt: rc.Config.Env["GITHUB_RUN_ATTEMPT"], + RunID: rc.Config.Env["GITHUB_RUN_ID"], + RunNumber: rc.Config.Env["GITHUB_RUN_NUMBER"], + Actor: rc.Config.Actor, EventName: rc.Config.EventName, - HeadRef: rc.Config.Env["GITHUB_HEAD_REF"], + Action: rc.CurrentStep, + Token: rc.Config.Token, Job: rc.Run.JobID, - Ref: rc.Config.Env["GITHUB_REF"], - RefName: rc.Config.Env["GITHUB_REF_NAME"], - RefType: rc.Config.Env["GITHUB_REF_TYPE"], - Repository: rc.Config.Env["GITHUB_REPOSITORY"], + ActionPath: rc.ActionPath, + ActionRepository: rc.Env["GITHUB_ACTION_REPOSITORY"], + ActionRef: rc.Env["GITHUB_ACTION_REF"], RepositoryOwner: rc.Config.Env["GITHUB_REPOSITORY_OWNER"], RetentionDays: rc.Config.Env["GITHUB_RETENTION_DAYS"], - RunAttempt: rc.Config.Env["GITHUB_RUN_ATTEMPT"], - RunID: rc.Config.Env["GITHUB_RUN_ID"], - RunNumber: rc.Config.Env["GITHUB_RUN_NUMBER"], RunnerPerflog: rc.Config.Env["RUNNER_PERFLOG"], RunnerTrackingID: rc.Config.Env["RUNNER_TRACKING_ID"], + Repository: rc.Config.Env["GITHUB_REPOSITORY"], + Ref: rc.Config.Env["GITHUB_REF"], Sha: rc.Config.Env["SHA_REF"], - Token: rc.Config.Token, - Workflow: rc.Run.Workflow.Name, + RefName: rc.Config.Env["GITHUB_REF_NAME"], + RefType: rc.Config.Env["GITHUB_REF_TYPE"], + BaseRef: rc.Config.Env["GITHUB_BASE_REF"], + HeadRef: rc.Config.Env["GITHUB_HEAD_REF"], Workspace: rc.Config.Env["GITHUB_WORKSPACE"], } if rc.JobContainer != nil { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 1755d595166..dd8afe54819 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -20,47 +20,47 @@ type Runner interface { // Config contains the config for a new runner type Config struct { - ActionCache ActionCache // Use a custom ActionCache Implementation + Actor string // the user that triggered the event + Workdir string // path to working directory ActionCacheDir string // path used for caching action contents ActionOfflineMode bool // when offline, use caching action contents - Actor string // the user that triggered the event - ArtifactServerAddr string // the address the artifact server binds to - ArtifactServerPath string // the path where the artifact server stores uploads - ArtifactServerPort string // the port the artifact server binds to - AutoRemove bool // controls if the container is automatically removed upon workflow completion BindWorkdir bool // bind the workdir to the job container - ContainerArchitecture string // Desired OS/architecture platform for running containers - ContainerCapAdd []string // list of kernel capabilities to add to the containers - ContainerCapDrop []string // list of kernel capabilities to remove from the containers - ContainerDaemonSocket string // Path to Docker daemon socket - ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) - ContainerOptions string // Options for the job container - DefaultBranch string // name of the main branch for this repository - Env map[string]string // env for containers EventName string // name of event to run EventPath string // path to JSON file to use for event.json in containers + DefaultBranch string // name of the main branch for this repository + ReuseContainers bool // reuse containers to maintain state ForcePull bool // force pulling of the image, even if already present ForceRebuild bool // force rebuilding local docker image action - GitHubInstance string // GitHub instance to use, default "github.com" - Inputs map[string]string // manually passed action inputs - InsecureSecrets bool // switch hiding output when printing to terminal - JSONLogger bool // use json or text logger LogOutput bool // log the output from docker run + JSONLogger bool // use json or text logger LogPrefixJobID bool // switches from the full job name to the job id - Matrix map[string]map[string]bool // Matrix config to run - NoSkipCheckout bool // do not skip actions/checkout + Env map[string]string // env for containers + Inputs map[string]string // manually passed action inputs + Secrets map[string]string // list of secrets + Vars map[string]string // list of vars + Token string // GitHub token + InsecureSecrets bool // switch hiding output when printing to terminal Platforms map[string]string // list of platforms Privileged bool // use privileged mode + UsernsMode string // user namespace to use + ContainerArchitecture string // Desired OS/architecture platform for running containers + ContainerDaemonSocket string // Path to Docker daemon socket + ContainerOptions string // Options for the job container + UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true + GitHubInstance string // GitHub instance to use, default "github.com" + ContainerCapAdd []string // list of kernel capabilities to add to the containers + ContainerCapDrop []string // list of kernel capabilities to remove from the containers + 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 + NoSkipCheckout bool // do not skip actions/checkout RemoteName string // remote name in local git repo config - ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub - ReuseContainers bool // reuse containers to maintain state - Secrets map[string]string // list of secrets - Token string // GitHub token - UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true - UsernsMode string // user namespace to use - Vars map[string]string // list of vars - Workdir string // path to working directory + ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. + Matrix map[string]map[string]bool // Matrix config to run + ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) + ActionCache ActionCache // Use a custom ActionCache Implementation } type caller struct {