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: parsing of tmt context input #137

Merged
merged 3 commits into from
Mar 26, 2024
Merged

fix: parsing of tmt context input #137

merged 3 commits into from
Mar 26, 2024

Conversation

jamacku
Copy link
Member

@jamacku jamacku commented Mar 21, 2024

@jamacku jamacku requested review from phracek and zmiklank March 21, 2024 14:07
@jamacku jamacku linked an issue Mar 21, 2024 that may be closed by this pull request
@zmiklank
Copy link
Collaborator

[test]

src/schema/input.ts Outdated Show resolved Hide resolved
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@           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           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zmiklank zmiklank merged commit 9840dac into sclorg:main Mar 26, 2024
7 checks passed
@henrywang
Copy link

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

@jamacku jamacku deleted the fix-tmt-context branch March 26, 2024 16:44
@zmiklank
Copy link
Collaborator

@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
Copy link
Collaborator

zmiklank commented Apr 5, 2024

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

@henrywang
Copy link

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

@henrywang
Copy link

henrywang commented Apr 5, 2024

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

@zmiklank
Copy link
Collaborator

zmiklank commented Apr 5, 2024

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

@henrywang
Copy link

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

Right.

@zmiklank
Copy link
Collaborator

zmiklank commented Apr 5, 2024

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
Copy link

henrywang commented Apr 5, 2024

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
Copy link
Collaborator

zmiklank commented Apr 5, 2024

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 added a commit to henrywang/bootc-workflow-test that referenced this pull request Apr 5, 2024
It's due to issue sclorg/testing-farm-as-github-action#137 (comment)

Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
@henrywang
Copy link

henrywang commented Apr 5, 2024

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

@henrywang
Copy link

@zmiklank I filed a new issue #152 to track this issue.

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.

v2 can't pass context to tmt
3 participants