search

sclorg / testing-farm-as-github-action

GitHub Action to execute tests by Testing Farm and update Pull Request status
MIT License
13 stars 11 forks source link

fix: parsing of tmt context input #137

Closed jamacku closed 5 months ago

jamacku commented 6 months ago

Fixes:

zmiklank commented 6 months ago

[test]

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.29%. Comparing base (a7d9cca) to head (c00dc72).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #137 +/- ## ======================================= Coverage 99.28% 99.29% ======================================= Files 8 8 Lines 563 566 +3 Branches 61 60 -1 ======================================= + Hits 559 562 +3 Misses 4 4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

henrywang commented 5 months ago

@zmiklank Thanks for fixing this issue. When will this fix be landed in v2? Thanks.

zmiklank commented 5 months ago

@henrywang We want to have a few days (a week) between landing new fix or feature and release. Meanwhile we test it on our repos. New tfaga could be released sometime next week then.

zmiklank commented 5 months ago

@henrywang Hello, this fix has just landed in v2.

henrywang commented 5 months ago

@zmiklank Thank you! I'll give new v2 a try.

henrywang commented 5 months ago

@zmiklank Hi! The context issue fixed. But I found the secret can't be passed to tmt. https://github.com/virt-s1/bootc-workflow-test/pull/220

zmiklank commented 5 months ago

@henrywang thanks for confirmation. Do you mean these secrets?

henrywang commented 5 months ago

@henrywang thanks for confirmation. Do you mean these secrets?

Right.

zmiklank commented 5 months ago

Interesting. I can see in pipeline.log in your test run, that secrets were passed in the request: http://artifacts.osci.redhat.com/testing-farm/f8f506d6-a23f-4f95-9d40-a0e88f2fe6f3/pipeline.log Also we have an integration test for checking that the secrets input works, and they are still passing: https://artifacts.dev.testing-farm.io/c8abe438-eba4-4052-84b1-fea4fa5f4e8c/

Can you maybe provide more information about this issue?

henrywang commented 5 months ago

The error log:

base64: invalid input
base64: invalid input
mv: cannot move '/tmp/tmp.66DLb9AKEw/client.conf' to '/etc/beaker/client.conf': No such file or directory
base64: invalid input

Those logs come from https://github.com/virt-s1/bootc-workflow-test/blob/main/tmt/tests/test.sh#L8. This is the action configuration: https://github.com/virt-s1/bootc-workflow-test/blob/05aa02c919b081f486af949c763081a3b90f6f56/.github/workflows/os-replace.yml#L90

zmiklank commented 5 months ago

It seems that decoding base64 ends with success even if its input is zero length string. Seems more like the binary string you want to pass to the base64 is invalid, not that the variable is empty. If I understand your script correctly, then the base64 command would not be executed at all, if the "secrets" variables were empty.

What is needed here is a minimal reproducer - something that does not incorporate various other programs that could fail. We have something like that here: https://github.com/sclorg/testing-farm-as-github-action/blob/main/.github/workflows/secrets_test.yml, but your issue is not reproducible with it.

henrywang commented 5 months ago

I rollback testing farm action to v1. Same PR test passed now https://github.com/virt-s1/bootc-workflow-test/pull/220. Looks like only some long string secrets affected.

henrywang commented 5 months ago

@zmiklank I filed a new issue https://github.com/sclorg/testing-farm-as-github-action/issues/152 to track this issue.