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

Move the signed commits check in our repos to the left in our process #1560

Open
2 tasks done
basiliskus opened this issue Nov 6, 2024 · 4 comments
Open
2 tasks done
Assignees
Labels
devex/opex A development excellence or operational excellence backlog item. Stream 2

Comments

@basiliskus
Copy link
Contributor

basiliskus commented Nov 6, 2024

DevEx/OpEx

One possible option is to add a pre-commit hook

Tasks

  • Adjust TI repository settings to disallow unsigned commits to all branches
  • Do the same for the SFTP ingestion repo

Additional Context

https://flexion.slack.com/archives/C055XTF22B0/p1727721609331709

@basiliskus basiliskus added the devex/opex A development excellence or operational excellence backlog item. label Nov 6, 2024
@jbiskie
Copy link
Contributor

jbiskie commented Dec 20, 2024

Upon researching, we found an issue with the current branch protection rules. The naming specification for the branch protection rules (In GitHub, under Settings->Branches), was not picking up all of our branches. As of the screenshot below, there were 34 branches in our repo, but the rules applied to only 14.

image

The rules use fnmatch (unix filename pattern matching), so "*" does not pick up any branches with a "/" slash in the name, as most of our branches use. Changing the catch-all pattern to "/" resolves this issue:

image

@jbiskie
Copy link
Contributor

jbiskie commented Dec 20, 2024

Secondly, all of us on the TI team were still able to push unsigned commits (to branches other than main) since we are all repo admins. The "Do not allow bypassing the above settings" option within the rule also needs to be checked. Without it, there's no warning or indication that we're pushing an unsigned commit.

image

@jbiskie
Copy link
Contributor

jbiskie commented Dec 20, 2024

These new settings have been applied to trusted-intermediary and reportstream-sftp-ingestion.

reportstream-sftp-ingestion only had the rule for main. The catch-all rule has been added so both repos' branch protection rules are consistent.

@halprin
Copy link
Member

halprin commented Jan 2, 2025

Thanks for looking into this! I never figured out what I was doing wrong to get the checks to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devex/opex A development excellence or operational excellence backlog item. Stream 2
Projects
None yet
Development

No branches or pull requests

4 participants