Skip to content

Commit

Permalink
Allow ignoring the presence of an OWNERS file (#390)
Browse files Browse the repository at this point in the history
When validating that a PR contains the correct set of file to certify a
chart, this PR adds the possibility to ignore the fact that an OWNERS
file is also present.

While an OWNERS file should never be present in this type of submission,
the case of an invalid Submission containing a mix of OWNERS and other
files is handled in the is_valid_owners_submission method.

Eventually, errors related to OWNERS file (which this belongs to) feed
the owners-error-message GITHUB OUTPUT, while other errors feed the
pr-content-error-message GITHUB OUTPUT.

Signed-off-by: Matthias Goerens <[email protected]>
  • Loading branch information
mgoerens authored Sep 30, 2024
1 parent 8c009dd commit 5f24f4f
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
11 changes: 9 additions & 2 deletions scripts/src/submission/submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,22 +360,29 @@ def set_tarball(self, file_path, tarball_match):
else:
self.modified_unknown.append(file_path)

def is_valid_certification_submission(self):
def is_valid_certification_submission(
self, ignore_owners: bool = False
) -> tuple[bool, str]:
"""Check wether the files in this Submission are valid to attempt to certify a Chart
We expect the user to provide either:
* Only a report file
* Only a chart - either as source or tarball
* Both the report and the chart
Note: While an OWNERS file should never be present in this type of submission, the case of
an invalid Submission containing a mix of OWNERS and other files is handled in the
is_valid_owners_submission method. The flag "ignore_owners" allows for skipping this
speficic check.
Returns False if:
* The user attempts to create the OWNERS file for its project.
* The PR contains additional files, not related to the Chart being submitted
Returns True in all other cases
"""
if self.modified_owners:
if self.modified_owners and not ignore_owners:
return False, "[ERROR] Send OWNERS file by itself in a separate PR."

if self.modified_unknown:
Expand Down
25 changes: 24 additions & 1 deletion scripts/src/submission/submission_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ class CertificationScenario:
input_submission: submission.Submission
expected_is_valid_certification: bool
expected_reason: str = ""
ignore_owners: bool = False


scenarios_certification_submission = [
Expand All @@ -303,6 +304,26 @@ class CertificationScenario:
expected_is_valid_certification=False,
expected_reason="[ERROR] Send OWNERS file by itself in a separate PR.",
),
# Invalid certification Submission contains OWNERS and report file, but ignore_owners is set to True
CertificationScenario(
input_submission=submission.Submission(
api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1",
modified_files=[
f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml"
f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS"
],
modified_owners=[
f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS"
],
report=submission.Report(
found=True,
signed=False,
path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml",
),
),
expected_is_valid_certification=True,
ignore_owners=True,
),
# Invalid certification Submission contains unknown files
CertificationScenario(
input_submission=submission.Submission(
Expand All @@ -319,7 +340,9 @@ class CertificationScenario:
@pytest.mark.parametrize("test_scenario", scenarios_certification_submission)
def test_is_valid_certification(test_scenario):
is_valid_certification, reason = (
test_scenario.input_submission.is_valid_certification_submission()
test_scenario.input_submission.is_valid_certification_submission(
test_scenario.ignore_owners
)
)
assert test_scenario.expected_is_valid_certification == is_valid_certification
assert test_scenario.expected_reason in reason
Expand Down

0 comments on commit 5f24f4f

Please sign in to comment.