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

Fix default value in verify-signed-rpms #1250

Closed

Conversation

lcarva
Copy link
Contributor

@lcarva lcarva commented Aug 5, 2024

The value of the FAIL_UNSIGNED parameter of the verify-signed-rpms Task is used when calling the rpm_verifier binary. An empty string value is not a valid value result in this error:

Usage: rpm_verifier [OPTIONS]
Try 'rpm_verifier --help' for help.

Error: Invalid value for '--fail-unsigned': '' is not a valid boolean.

This commit changes the default value to "false". This is both valid and inline with ADR-14.

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

The value of the `FAIL_UNSIGNED` parameter of the `verify-signed-rpms`
Task is used when calling the `rpm_verifier` binary. An empty string
value is not a valid value result in this error:

```
Usage: rpm_verifier [OPTIONS]
Try 'rpm_verifier --help' for help.

Error: Invalid value for '--fail-unsigned': '' is not a valid boolean.
```

This commit changes the default value to `"false"`. This is both valid
and inline with
[ADR-14](https://konflux-ci.dev/architecture/ADR/0014-let-pipelines-proceed.html).

Signed-off-by: Luiz Carvalho <[email protected]>
@lcarva lcarva requested a review from dirgim August 5, 2024 20:20
@yftacherzog
Copy link
Member

This task is maintained in tekton-tools and replicated here, so changes should be made there, where it's also tested.
We should probably mention that on the README.
https://github.com/redhat-appstudio/tekton-tools/blob/main/tasks/verify-signed-rpms/0.1/verify-signed-rpms.yaml

@lcarva
Copy link
Contributor Author

lcarva commented Aug 6, 2024

This task is maintained in tekton-tools and replicated here, so changes should be made there, where it's also tested. We should probably mention that on the README. https://github.com/redhat-appstudio/tekton-tools/blob/main/tasks/verify-signed-rpms/0.1/verify-signed-rpms.yaml

Thanks for the info! That is a private repo with forking disabled. I cannot contribute.

@yftacherzog
Copy link
Member

This task is maintained in tekton-tools and replicated here, so changes should be made there, where it's also tested. We should probably mention that on the README. https://github.com/redhat-appstudio/tekton-tools/blob/main/tasks/verify-signed-rpms/0.1/verify-signed-rpms.yaml

Thanks for the info! That is a private repo with forking disabled. I cannot contribute.

I added you. Let me know if you're now able to create a branch on the repo itself and create a PR.

@lcarva
Copy link
Contributor Author

lcarva commented Aug 7, 2024

@lcarva lcarva closed this Aug 7, 2024
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.

2 participants