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 a ruletype for enforcing a file being present in a repo #227

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

Conversation

rdimitrov
Copy link
Member

@rdimitrov rdimitrov commented Dec 17, 2024

The following PR creates a ruletype that lets you ensure you have a certain file in your repository, but also allows you to specify its expected content.

This rule is useful for enforcing the presence of various files in the repository, such as LICENSE, README, CONTRIBUTING,
CODE_OF_CONDUCT, Dependabot configuration files and many more. It can also be used to enforce the presence of specific
content in the file so you know that the file is not just a placeholder.

Depends on mindersec/minder#5216

This ruletype also implements OSPS-DO-02

@rdimitrov rdimitrov requested a review from a team as a code owner December 17, 2024 13:40
@rdimitrov rdimitrov self-assigned this Dec 17, 2024
Signed-off-by: Radoslav Dimitrov <[email protected]>
Copy link
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

A few nits, but seems good overall.

rule-types/common/enforce_file.yaml Outdated Show resolved Hide resolved
rule-types/common/enforce_file.yaml Outdated Show resolved Hide resolved
Ensures that the repository contains a specific file and its content.

This rule is useful for enforcing the presence of various files in the repository, such as LICENSE, README, CONTRIBUTING,
CODE_OF_CONDUCT, Dependabot configuration files and many more. It can also be used to enforce the presence of specific
Copy link
Member

Choose a reason for hiding this comment

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

I suspect that Dependabot is trickier, because you probably want it to be adjusted based on the languages present. The others seem fairly likely to be consistent.

Comment on lines 24 to 39
file:
type: string
description: |
The file to enforce in the repository.
For example, LICENSE, README, CONTRIBUTING, .github/dependabot.yml, etc.
default: LICENSE
content:
type: string
description: |
The content to enforce in the file.
For example, the content of the LICENSE file.

Leave "" to only check for the presence of the license file.
required:
- file
- content
Copy link
Member

Choose a reason for hiding this comment

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

Question: right now, it looks like you'd end up needing to specify N copies of the rule for N different files.

What would you think of having a schema like the following?

Suggested change
file:
type: string
description: |
The file to enforce in the repository.
For example, LICENSE, README, CONTRIBUTING, .github/dependabot.yml, etc.
default: LICENSE
content:
type: string
description: |
The content to enforce in the file.
For example, the content of the LICENSE file.
Leave "" to only check for the presence of the license file.
required:
- file
- content
files:
type: object
additionalProperties:
type: string
description: |
A mapping from file path to expected contents in the repository
Set expected contents to "" (empty string) to check for file existence without
checking file contents.
default:
required:
- files

rule-types/common/enforce_file.yaml Outdated Show resolved Hide resolved
Comment on lines +74 to +85
type: pull_request
pull_request:
title: "Ensure {{.Profile.file }} exists with the expected content"
body: |
This is a Minder automated pull request.

This pull request ensures that this repository contains the file {{.Profile.file}} with the expected content set by your organization.
contents:
- path: "{{.Profile.file}}"
action: replace
content: |
{{.Profile.content}}
Copy link
Member

Choose a reason for hiding this comment

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

Ah! This is a (current) reason not to use the object formulation -- we don't have a way to iterate over all the keys in the Profile.files map, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly 👍 Of course, we can try to do that if we want. I was also thinking if I should try and make the path field optional for the pull request remediation type, but at the same time I like explicitly scoping it so you don't accidentally push another change. My concern for not looking at these is both will be considered breaking changes so I think it'll be out of scope for this one.

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

Successfully merging this pull request may close these issues.

2 participants