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

Add workflow to synchronize go mod changes with submodules #314

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

Conversation

codysoyland
Copy link
Member

@codysoyland codysoyland commented Oct 14, 2024

Summary

This PR attempt to improve the workflow of managing Dependabot updates to this repo. The problem is that when Dependabot updates a dependency in the main module, those updates are not reflected in the "submodule" (https://github.com/sigstore/sigstore-go/tree/main/examples/oci-image-verification) which includes a reference to the main module. That causes the build to fail, and one maintainer must manually run go mod tidy in the submodule and push those changes, and another maintainer must approve and merge them (as the last committer is disallowed from merging a PR). (example)

This PR automatically runs go mod tidy in both modules and commits/pushes them whenever Dependabot creates a new PR.

There is a caveat with this, which is that the actions do not run when the last committer used GITHUB_TOKEN to authenticate to the Git server, which is intentional to avoid accidentally creating recursively invoked workflows. This can be worked around by using a PAT in this workflow to authenticate to the Git server, or maintainers can push an empty commit to trigger the Actions workflows.

If this seems like a good approach to fellow maintainers, I'd recommend we create a PAT for use within this repo and modify this workflow to use it.

This workflow could also be added to sigstore/sigstore for use with the KMS providers (example).

Release Note

Documentation

@codysoyland codysoyland marked this pull request as ready for review October 14, 2024 18:22
@codysoyland codysoyland requested a review from a team as a code owner October 14, 2024 18:22
Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

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

Thanks Cody! This looks good.
I think it may even work to just re-run the failing workflows from the UI instead of pushing an empty commit?

@codysoyland
Copy link
Member Author

I think it may even work to just re-run the failing workflows from the UI instead of pushing an empty commit?

I don't think there's a way to trigger the workflows from the UI if they are not run due to the last commit originating from Actions -- they are not actually "failed" workflows, they just aren't created at all. Maybe I'm missing something, but I think it would require an empty commit to trigger the build, unless we add a PAT to a secret and use it to commit the go.mod changes.

@haydentherapper (or maybe @cpanato?), what do you think of incorporating this workflow and using a PAT to authenticate with git? As explained in the description, we need to use a PAT or the actions-generated commit will not trigger actions to run again, therefore the automation would be incomplete. We can create an org-level PAT secret and also use it to keep the kms providers in sigstore/sigstore up to date using the same workflow. Somebody who is org admin would need to create the PAT and upload it as an org-level secret.

@haydentherapper
Copy link
Contributor

I really like this workflow, this is great! This has definitely been a nuisance here and in Sigstore/sigstore.

PATs are a little annoying to keep up to date, but we’ve already got a number that we manage, one more is fine. In the future, we can look at creating a GitHub app to replace PATs.

what are the permissions needed for the PAT?


jobs:
synchronize-go-mod:
if: github.actor == 'dependabot[bot]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an ID? If someone names their account this string, this would run. Not really a big deal though

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the documented way. The actor name is a user name of the entity that triggered the workflow.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with Hayden's question here - having this pinned to an immutable ID feels marginally safer.

- name: Install Go
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2
with:
go-version-file: go.mod
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the modules don’t use the same version of go? Should we search for all go.mod files, then install go and run go mod tidy for each?

Copy link
Member Author

Choose a reason for hiding this comment

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

I hadn't considered that, but I think we'd hit an error in that case, and we'd need to manually sync up the go versions.

Copy link
Member

@kommendorkapten kommendorkapten Oct 16, 2024

Choose a reason for hiding this comment

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

If we create a go.work and workspace, we can define the go version at the top level and use go-version-file: go.work, then rely on go work sync to propagate the version to the sub modules. That could probably be part of this workflow too.

@codysoyland
Copy link
Member Author

PATs are a little annoying to keep up to date, but we’ve already got a number that we manage, one more is fine. In the future, we can look at creating a GitHub app to replace PATs.

Yeah an app is ideal, just requires a little more setup. PATs are indeed annoying.

what are the permissions needed for the PAT?

It can be a fine-grained permission PAT with support to the specific repos we need (sigstore, sigstore-go), with "Contents" set to "Read and write".

@haydentherapper
Copy link
Contributor

@cpanato can you create the PAT?

@haydentherapper
Copy link
Contributor

Coming back to this, I can create the PAT...but with that said, keeping these PATs up to date is a bit annoying as they expire. Is there any way to do this without another PAT?

@codysoyland
Copy link
Member Author

Coming back to this, I can create the PAT...but with that said, keeping these PATs up to date is a bit annoying as they expire. Is there any way to do this without another PAT?

I believe we can do this with a GitHub app.... I can test it on my own org first to make sure and help you set it up if that works!

@haydentherapper
Copy link
Contributor

We had also looked into https://github.com/apps/octo-sts awhile ago which Chainguard made.

@cpanato
Copy link
Member

cpanato commented Oct 29, 2024

sorry for the delay here

we can create a PAT, but i would vote to use the octo-sts, because we dont need to manage the PAT and that will be valid only for the job execution

@cpanato
Copy link
Member

cpanato commented Oct 29, 2024

and this looks soo cool!!

@codysoyland
Copy link
Member Author

octo-sts is awesome, I just tried it on a test repo and it works great for this purpose. I'll update the PR

@haydentherapper
Copy link
Contributor

We'll need to install it in the org as well. Should mention this to the TSC before we progress.

Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland
Copy link
Member Author

I updated the workflow to use octo-sts, so if TSC approves, we can proceed to merging this.

@haydentherapper
Copy link
Contributor

@codysoyland Can you file an issue on https://github.com/sigstore/tsc asking for approval?

@codysoyland
Copy link
Member Author

@codysoyland Can you file an issue on https://github.com/sigstore/tsc asking for approval?

sigstore/TSC#51


- name: Synchronize go.mod in submodule
run: |
cd examples/oci-image-verification; go mod tidy
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cd examples/oci-image-verification; go mod tidy
cd examples/oci-image-verification && go mod tidy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants