Initial implementation of the frizbee action by rdimitrov · Pull Request #2 · stacklok/frizbee-action

rdimitrov · 2024-06-10T23:15:18Z

Initial implementation of the Frizbee Action.

Details:

Supports GitHub Actions, Dockerfiles and all yaml/yml files containing known container image references, i.e. docker-compose files, Kubernetes files, etc.
Supports opening PRs suggesting repository owners to pin their actions or container images

Todo:

The default GITHUB_TOKEN granted from the action run does not have the workflows permissions which limits the action to open PRs against the workflow files. To resolve this, the user would have to create a new token with the correct permissions and assign it to the action.
Support re-running the action if there's previous PR created
Test it from another repo when it's published - stacklok/frizbee-action@v0.0.1
There's a test.yml workflow and a tests folder with a bunch of example files. Let me know if you want these deleted now or we can keep them until we are okay with the functionality of the initial version.
Add support for checking the content of the PR instead of the whole repository

Fixes: stacklok/frizbee#149

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

rdimitrov · 2024-06-11T11:54:25Z

action.yml

  icon: "at-sign"
  color: "green"
 inputs:
-  GITHUB_TOKEN:


This is already there from the GITHUB environment

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

lukehinds · 2024-06-11T12:53:37Z

tests/docker_compose/docker-compose.yaml

Is this checked in accidently? We should not need minder

The tests.yml workflow and everything under the tests directory are test files to test the action with. My thought was to leave them there until we make this ready and remove them afterwards. I should have mentioned this explicitly.

lukehinds · 2024-06-11T12:55:42Z

main.go

+}
+
+// cloneRepository clones the repository and returns a billy.Filesystem interface to interact with it
+func cloneRepository(url, owner, accessToken string) (billy.Filesystem, *git.Repository, error) {


I wonder if there is an alternative approach to cloning? This might take a long time if there are large blobs in the repo.

I guess a lot of this is based on the approach, is this intercepting PR's or a workflow that is scheduled?

The use case for this is when the actions runs from a scheduled run. We can do something else once we add support for checking the contents of a PR patch too.

Intercepting the PRs would have the added benefit of not requiring as high privileges I think.

About cloning, I view it as acceptable for because the action is not typically run on infra we pay for :-) we can then optimize the clone with something like git sparse checkout since the action configuration already tells us which files or directories to look at.

lukehinds · 2024-06-11T12:57:28Z

action.yml

+    description: "Docker Compose files to correct"
+    required: false
+    default: ""
+  open_pr:


what would happen if open_pr is false, I guess it would never work?

If open_pr is false it would run the action, parse the files, and output what it did, perhaps fail the run if fail_on_unpinned is set, but it won't create a PR.

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

rdimitrov · 2024-06-11T13:10:01Z

The Trivy scan failure is because of an indirect dependency.

rdimitrov · 2024-06-11T13:21:53Z

Example PR:

Frizbee: Pin images and actions to commit hash rdimitrov/frizbee-action#17

Example test runs with different configurations -

https://github.com/rdimitrov/frizbee-action/actions/runs/9466003011 - Note there was already a PR created so the action did not had to create one)
https://github.com/rdimitrov/frizbee-action/actions/runs/9465453910/job/26075006256 - One where the PR was created

jhrozek · 2024-06-11T14:54:31Z

The Trivy scan failure is because of an indirect dependency.

if the dep can't be updated, then we need to add a trivy ignore (let me know if you want me to do that)

jhrozek · 2024-06-11T15:05:17Z

Dockerfile

@@ -1,5 +1,4 @@
 FROM golang:alpine3.19@sha256:0466223b8544fb7d4ff04748acc4d75a608234bf4e79563bff208d2060c0dd79


do you know if dependabot automatically bumps these? I think it's easier to use the golang:goversion images to see what go version are we using (I don't particulary care about the OS version, we're not installing anything anyway)

jhrozek · 2024-06-11T15:07:02Z

README.md

+  dockerfiles:
+    description: "Dockerfiles to correct"
+    required: false
+    default: "Dockerfile"


as discussed on meet, I think a list of files or dirs would be more useful, e.g. in minder we have:

./docker/reminder/Dockerfile ./docker/minder/Dockerfile

but the default is sensible, great!

jhrozek · 2024-06-11T15:08:25Z

README.md

+
+The default `GITHUB_TOKEN` doesn't have the necessary permissions (`workflows`) to open a PR. 
+In case you want to use the `open_pr` feature, you will need to create a new token with the correct scope, add it as a secret
+and pass it to the action through the `GITHUB_TOKEN` environment variable.


do we have a way to tell the user in a nice way (some introspection of the token, e.g. hitting the /user endpoint with it)

jhrozek · 2024-06-11T15:08:54Z

main.go

+import (
+	"context"
+	"errors"
+	"fmt"


for later: we should lint the code before it gets too big

jhrozek · 2024-06-11T15:09:45Z

main.go

+
 func main() {
-	// This is a placeholder for the main function
+	ctx := context.Background()


do you think it makes sense to provide a sensible timeout here (a couple of minutes maybe?) to avoid running the action for too long? Or is it better set somewhere in the action config?

jhrozek · 2024-06-11T15:12:01Z

main.go

+// 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")


would it make testing the function later easier to pass the tokens etc as parameters?

jhrozek · 2024-06-11T15:14:07Z

main.go

+	}
+
+	// Clone the repository
+	fs, repo, err := cloneRepository(fmt.Sprintf("https://github.com/%s", repoFullName), repoOwner, token)


not sure if there's any use-case, but we might want to use the GH API to get the clone URL rather than constructing it ourselves.

jhrozek · 2024-06-11T15:20:11Z

pkg/action/action.go

+			log.Fatalf("failed to close file %s: %v", path, err) // nolint:errcheck
+		}
+	}()
+	_, err = fmt.Fprintf(f, "%s", content)


why not io.WriteString and skip the formatting parameter?

jhrozek · 2024-06-11T15:20:48Z

pkg/action/action.go

+	// Commit the change
+	_, err = worktree.Commit(fmt.Sprintf("Update %s by pinning its image references", path), &git.CommitOptions{
+		Author: &object.Signature{
+			Name:  "github-actions[bot]",


should we use a specific author name?

jhrozek · 2024-06-11T15:21:08Z

pkg/action/action.go

+// createPR creates a new pull request with the changes made so far
+func (fa *FrizbeeAction) createPR(ctx context.Context) error {
+	// Create a new branch for the PR
+	branchName := "frizbee-action-patch"


jhrozek · 2024-06-11T15:21:43Z

pkg/action/action.go

+	for _, pr := range openPrs {
+		if pr.GetHead().GetRef() == branchName {
+			fmt.Printf("PR %d already exists\n", pr.GetNumber())
+			return nil


shouldn't we update a PR if it exists?

jhrozek

I left a couple of comments inline, but I say let's merge this, get the codebase rolling and then iterate (we can either go back to the comments here and just reply to them or create issues)

lukehinds · 2024-06-11T17:19:00Z

I am happy if you folks think its is in decent shape. I have a feeling I prefer a scheduled run dependabot style type experience as default, but happy to discuss more if need be.

rdimitrov · 2024-06-11T21:28:35Z

Let's merge this so we can get it rolling and I'll file separate issues for the comments @lukehinds and @jhrozek made so we keep and address their feedback 👍 Thank you both!

rdimitrov force-pushed the implement-frizbee-action branch 3 times, most recently from 1eb5a37 to 6e737d6 Compare June 11, 2024 08:42

Initial implementation of the Frizbee action

5730761

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

rdimitrov force-pushed the implement-frizbee-action branch 6 times, most recently from 76f5bef to 16688a0 Compare June 11, 2024 11:24

rdimitrov marked this pull request as ready for review June 11, 2024 11:24

rdimitrov changed the title ~~[Draft] Initial implementation of the frizbee action~~ Initial implementation of the frizbee action Jun 11, 2024

rdimitrov force-pushed the implement-frizbee-action branch 8 times, most recently from 11d6634 to ce0a2d7 Compare June 11, 2024 11:52

rdimitrov commented Jun 11, 2024

View reviewed changes

action.yml

icon: "at-sign"

color: "green"

inputs:

GITHUB_TOKEN:

Copy link

Member Author

rdimitrov Jun 11, 2024

Choose a reason for hiding this comment

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

This is already there from the GITHUB environment

Use go-git instead

5898d4f

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

rdimitrov force-pushed the implement-frizbee-action branch from ce0a2d7 to 5898d4f Compare June 11, 2024 12:23

lukehinds reviewed Jun 11, 2024

View reviewed changes

Add README.md and CONTRIBUTING.md

5602ab2

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>

rdimitrov force-pushed the implement-frizbee-action branch from cf31c95 to 5602ab2 Compare June 11, 2024 12:58

jhrozek reviewed Jun 11, 2024

View reviewed changes

main.go

import (

"context"

"errors"

"fmt"

Copy link

Contributor

jhrozek Jun 11, 2024

Choose a reason for hiding this comment

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

for later: we should lint the code before it gets too big

jhrozek reviewed Jun 11, 2024

View reviewed changes

jhrozek approved these changes Jun 11, 2024

View reviewed changes

rdimitrov merged commit 327fadc into stacklok:main Jun 11, 2024

rdimitrov deleted the implement-frizbee-action branch June 11, 2024 21:29

This was referenced Jun 11, 2024

Return a better message when GITHUB_TOKEN lacks the right permissions (workflows) #9

Open

Use GH's API to construct the repo URL instead of fmt.Sprintf #12

Open

Update the commit's author #13

Open

		@@ -1,5 +1,4 @@
		FROM golang:alpine3.19@sha256:0466223b8544fb7d4ff04748acc4d75a608234bf4e79563bff208d2060c0dd79

Conversation

rdimitrov commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdimitrov commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rdimitrov commented Jun 11, 2024

Uh oh!

jhrozek commented Jun 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhrozek left a comment

Choose a reason for hiding this comment

Uh oh!

lukehinds commented Jun 11, 2024

Uh oh!

rdimitrov commented Jun 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rdimitrov commented Jun 10, 2024 •

edited

Loading

rdimitrov commented Jun 11, 2024 •

edited

Loading