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.

Sign up for GitHub

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

Jump to bottom

Allow running in place on an existing checkout #53

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Allow running in place on an existing checkout #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db8bf69
Allow running in place on an existing checkout
segiddins Sep 10, 2024
8073635
Update golang base image
segiddins Sep 10, 2024
850cb44
Remove unneeded check for empty paths
segiddins Sep 10, 2024
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
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:alpine3.19@sha256:0466223b8544fb7d4ff04748acc4d75a608234bf4e79563bff208d2060c0dd79
FROM golang:1.23.1-alpine3.20@sha256:ac67716dd016429be8d4c2c53a248d7bcdf06d34127d3dc451bda6aa5a87bc06

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we include this change in another PR? I think it's a good change.

COPY . /home/src
WORKDIR /home/src
Expand Down
6 changes: 5 additions & 1 deletion action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ inputs:
actions:
description: "Actions to correct"
required: false
default: ".github/workflows"
default: '[".github/workflows"]'
Copy link

Choose a reason for hiding this comment

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

Hey @segiddins thank you for your contributio!
I'm not an expert here, but this looks like a breaking change, how would this impact existing users of the action?
cc @evankanderson

dockerfiles:
description: "Dockerfiles to correct"
required: false
Expand All @@ -29,6 +29,10 @@ inputs:
description: "Fail if an unpinned action/image is found"
required: false
default: "false"
in_place:
description: "Update the files in place"
required: false
default: "false"
Comment on lines +32 to +35
Copy link
Member

Choose a reason for hiding this comment

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

So, I agree that the existing action is a bit weird -- what would you think about making this be a directory path argument, where the default "" value means to do the existing fetch-from-github behavior? i.e.

Suggested change
in_place:
description: "Update the files in place"
required: false
default: "false"
repo_root:
description: "Operate on files in the specified filesystem location. If unspecified, check out files from the current repo."
required: false
default: ""

runs:
using: "docker"
image: "Dockerfile"
Expand Down
47 changes: 32 additions & 15 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

"github.com/go-git/go-billy/v5"
"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-billy/v5/osfs"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing/transport/http"
"github.com/go-git/go-git/v5/storage/memory"
Expand Down Expand Up @@ -59,15 +60,10 @@

// initAction initializes the frizbee action - reads the environment variables, creates the GitHub client, etc.
func initAction(ctx context.Context) (*action.FrizbeeAction, error) {
// Get the GitHub token from the environment
token := os.Getenv("GITHUB_TOKEN")
if token == "" {
return nil, errors.New("GITHUB_TOKEN environment variable is not set")
}

// Create a new GitHub client
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
tc := oauth2.NewClient(ctx, ts)
var repo *git.Repository
var fs billy.Filesystem
var githubClient *github.Client
var token string
Comment on lines +63 to +66
Copy link
Member

Choose a reason for hiding this comment

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

It looks like a bunch of these (particularly githubClient and token) don't get initialized on the "directory" path. Should we remove them?


// Get the GITHUB_REPOSITORY_OWNER
repoOwner := os.Getenv("GITHUB_REPOSITORY_OWNER")
Expand All @@ -81,10 +77,31 @@
return nil, errors.New("GITHUB_REPOSITORY environment variable is not set")
}

// Clone the repository
fs, repo, err := cloneRepository("https://github.com/"+repoFullName, repoOwner, token)
if err != nil {
return nil, fmt.Errorf("failed to clone repository: %w", err)
if os.Getenv("INPUT_IN_PLACE") != "true" {

Check failure on line 80 in main.go

View workflow job for this annotation

GitHub Actions / lint 

string `true` has 3 occurrences, make it a constant (goconst)
// Get the GitHub token from the environment
token = os.Getenv("GITHUB_TOKEN")
if token == "" {
return nil, errors.New("GITHUB_TOKEN environment variable is not set")
}

// Create a new GitHub client
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
tc := oauth2.NewClient(ctx, ts)
githubClient = github.NewClient(tc)

// Clone the repository
var err error
fs, repo, err = cloneRepository("https://github.com/"+repoFullName, repoOwner, token)
if err != nil {
return nil, fmt.Errorf("failed to clone repository: %w", err)
}
} else {
fs = osfs.New(".")
var err error
repo, err = git.PlainOpen(".")
if err != nil {
return nil, fmt.Errorf("failed to open repository: %w", err)
}
}

cfg := config.DefaultConfig()
Expand All @@ -107,12 +124,12 @@

// Read the action settings from the environment and create the new frizbee replacers for actions and images
return &action.FrizbeeAction{
Client: github.NewClient(tc),
Client: githubClient,
Token: token,
RepoOwner: repoOwner,
RepoName: strings.TrimPrefix(repoFullName, repoOwner+"/"),

ActionsPath: os.Getenv("INPUT_ACTIONS"),
ActionsPaths: envToStrings("INPUT_ACTIONS"),
Comment on lines -115 to +132
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to add a second argument which covers the multiple-paths option (which could be exclusive with the single argument), OR to change valToStrings to also interpret the input foo as equivalent to ["foo"]. (To be honest, I'd rather have had comma-delimited strings over JSON elsewhere, but that ship has sailed.)

Copy link
Member

Choose a reason for hiding this comment

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

Thinking further, I converting foo to ["foo"] is dangerous, because we might end up with ["[\"foo\",\n]" depending on how strict the JSON parser is and how lazy the user is. Maybe add a second argument for multiple GitHub Actions directories?

DockerfilesPaths: envToStrings("INPUT_DOCKERFILES"),
KubernetesPaths: envToStrings("INPUT_KUBERNETES"),
DockerComposePaths: envToStrings("INPUT_DOCKER_COMPOSE"),
Expand Down
42 changes: 19 additions & 23 deletions pkg/action/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type FrizbeeAction struct {
RepoOwner string
RepoName string

ActionsPath string
ActionsPaths []string
DockerfilesPaths []string
KubernetesPaths []string
DockerComposePaths []string
Expand All @@ -53,7 +53,6 @@ type FrizbeeAction struct {
ImagesReplacer *replacer.Replacer
BFS billy.Filesystem
Repo *git.Repository
bodyBuilder *strings.Builder
Copy link
Member

Choose a reason for hiding this comment

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

Removing bodyBuilder seems like a (nice) simple refactor which doesn't change behavior. I could approve that change real fast if it wasn't entangled in the behavior change around ActionsPaths changing contents.

}

// Run runs the frizbee action
Expand All @@ -77,21 +76,18 @@ func (fa *FrizbeeAction) Run(ctx context.Context) error {

// parseWorkflowActions parses the GitHub Actions workflow files
func (fa *FrizbeeAction) parseWorkflowActions(ctx context.Context, out *replacer.ReplaceResult) error {
if fa.ActionsPath == "" {
log.Printf("Workflow path is empty")
return nil
}

log.Printf("Parsing workflow files in %s...", fa.ActionsPath)
res, err := fa.ActionsReplacer.ParsePathInFS(ctx, fa.BFS, fa.ActionsPath)
if err != nil {
return fmt.Errorf("failed to parse workflow files in %s: %w", fa.ActionsPath, err)
}
for _, path := range fa.ActionsPaths {
log.Printf("Parsing workflow files in %s...", path)
res, err := fa.ActionsReplacer.ParsePathInFS(ctx, fa.BFS, path)
if err != nil {
return fmt.Errorf("failed to parse workflow files in %s: %w", path, err)
}

// Copy the processed and modified files to the output
out.Processed = mapset.NewSet(out.Processed...).Union(mapset.NewSet(res.Processed...)).ToSlice()
for key, value := range res.Modified {
out.Modified[key] = value
// Copy the processed and modified files to the output
out.Processed = mapset.NewSet(out.Processed...).Union(mapset.NewSet(res.Processed...)).ToSlice()
for key, value := range res.Modified {
out.Modified[key] = value
}
}
return nil
}
Expand Down Expand Up @@ -256,21 +252,21 @@ func (fa *FrizbeeAction) createPR(ctx context.Context) error {
}
defaultBranch := repository.GetDefaultBranch()

fa.bodyBuilder = &strings.Builder{}
fa.bodyBuilder.WriteString("## Frizbee: Pin images and actions to commit hash\n\n")
fa.bodyBuilder.WriteString("The following PR pins images and actions to their commit hash.\n\n")
fa.bodyBuilder.WriteString("Pinning images and actions to their commit hash ensures that the same " +
bodyBuilder := &strings.Builder{}
bodyBuilder.WriteString("## Frizbee: Pin images and actions to commit hash\n\n")
bodyBuilder.WriteString("The following PR pins images and actions to their commit hash.\n\n")
bodyBuilder.WriteString("Pinning images and actions to their commit hash ensures that the same " +
"version of the image or action is used every time the workflow runs. This is important for " +
"reproducibility and security.\n\n")
//nolint:lll
fa.bodyBuilder.WriteString("Pinning is a [security practice recommended by GitHub](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions).\n\n")
bodyBuilder.WriteString("Pinning is a [security practice recommended by GitHub](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions).\n\n")
//nolint:lll
fa.bodyBuilder.WriteString("🥏 Posted on behalf of [frizbee-action](https://github.com/stacklok/frizbee-action) 🥏, by [Stacklok](https://stacklok.com).\n\n")
bodyBuilder.WriteString("🥏 Posted on behalf of [frizbee-action](https://github.com/stacklok/frizbee-action) 🥏, by [Stacklok](https://stacklok.com).\n\n")

// Create a new PR
pr, _, err := fa.Client.PullRequests.Create(ctx, fa.RepoOwner, fa.RepoName, &github.NewPullRequest{
Title: github.String("Frizbee: Pin images and actions to commit hash"),
Body: github.String(fa.bodyBuilder.String()),
Body: github.String(bodyBuilder.String()),
Head: github.String(branchName),
Base: github.String(defaultBranch),
MaintainerCanModify: github.Bool(true),
Expand Down
Loading