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

RFE: prepare part for each step in plan #588

Open
Tracked by #15431
jscotka opened this issue Feb 25, 2021 · 16 comments
Open
Tracked by #15431

RFE: prepare part for each step in plan #588

jscotka opened this issue Feb 25, 2021 · 16 comments

Comments

@jscotka
Copy link
Collaborator

jscotka commented Feb 25, 2021

I had trouble with order of commands that prepare is done after discovery. (Or better to say, it is clear, that prepare is executed on target machine but not so clean that discovery not and order of these commands ). It would good to do some steps before discovery.

My usecase is, that I have no FMF data for tests, just plan, and tests metadata are automatically generated from python decorators. So that I had to run my tool what generates them, and discovery is done before it tries to do that. and do not want to store these automatically generated fmf files inside git, as potentialy of out of sync.

as we've discussed with @lukaszachy it probably makes sense that each step could have some possibility to run some setup before running the rest, some setup or assemble attribute with same syntax like current prepare (or maybe allow just script?)

syntax could be like

discover:
    setup: (do commands on host machine if discover is done on host)
       - name: install deps on host
         how: install
         packages:
            a
            b
        - name: generate fmf data
          script: ./path/to/script
    how: fmf
execute:
    setup: (do commands on guest machine)
      ... 
    how: tmt

My current plan

It expected that prepare is before discovery (and expected that discovery is also done on target machine)

summary: Run browser integration tests on the host
prepare:
  - name: Install cockpit and test dependencies
    how: install
    package:
    - cockpit
    - cockpit-ws
    - cockpit-system
    - cockpit-bridge
    - git
    - libvirt-python3
    - make
    - npm
    - python3
  - name: Install Chromium
    script: ./plans/prepare-chromium.sh
  - name: Create users for tests
    script: ./plans/prepare-users.sh
  - name: Start cockpit service & generic hacks
    script: ./plans/prepare-generic.sh
  - name: Regenerate fmf data for tests form python files
    script: ./plans/prepare-fmf-regeneration.sh
discover:
  how: fmf
  test: [/verify/check-testlib/TestRunTestListing/testBasic]
execute:
  how: tmt

I've tried to use some ughly hack like:

discover:
  - name: Regenerate fmf data for tests form python files
    test: ./plans/prepare-fmf-regeneration.sh
    how: shell
  - name: discover rest
    how: fmf
    test: [/verify/check-testlib/TestRunTestListing/testBasic]

But it does not work, or better to say, unable to find output of the first discovery step, script output is not in log.txt not anywhere, so it is possible that something is bad in my syntax, but cannot see what`s wrong.
But this is surely separate issue.

@martinpitt
Copy link

This is similar to issue #585 which I reported yesterday. This issue is a bit more general, so I suppose pick which proposal you like better, and then close one or the other. Thanks!

@jkonecny12
Copy link

I like this solution but I don't think it should be part of the discover step. I would be more inclined to have something like pre-prepare or similar which would be separate from the Discover.

@pvalena
Copy link
Collaborator

pvalena commented Mar 26, 2021

I think this goes against tmt approach, which is, not modifying anything in current repository (that is I'd be fine with it if it's in /var/tmp/tmt). The local host modification is IMO no-go (unless local provisioner is selected). I don't think TMT ever aimed at replacing Makefile (f.e. for building a tarball) or guest setup (f.e. setup.sh); or ansible for that matter. TMT is managing "tests"; or manage them, not tarballs / sources / host system. That is tests metadata, not "development" metadata (unless you consider L3 being that).

I'd really love keeping tmt run safe to run on my system (with default provision). That way, nobody can inject rm -rf in some upstream project, or link it via wget, or load some other unknown payload.


Note: I know that the "discover: how: shell" may be unsafe as well, but then again, that's not the default, nor is that the intended use, and I hope I won't see it widely used if it's enabled, but I'm afraid that can't be said for any pre-prepare, if we implement it.

@martinpitt
Copy link

@pvalena : Agreed that this should really not modify anything in the current repository. I also don't think that this was the proposal. All of these scripts should run in /var/tmp/tmp/run-*/discover/ only. But I do see the concern about running too much free-form code in the metadata, that is both brittle and unsafe. So maybe this should be dialed back to more specific use cases, and support them in a declarative way.

@pvalena
Copy link
Collaborator

pvalena commented Apr 14, 2021

@martinpitt when the arbitrary script runs on the host it has access into /home and everywhere else... if you give it sudo (for installation; or it gains it from tmt by some means), it can also fully modify the host system. I agree, I don't see any other way than the support of very specific cases in declarative manner.

E.g. not running make but rather the end command itself tar .... And even then it should verify the paths passed to tar (input/output files).

@pvalena
Copy link
Collaborator

pvalena commented Apr 14, 2021

Running it in container or such (same method as provision) would be an option, and maybe a better one, as it would support the free-form. I'm not sure how this would be implemented in CI though. @thrix WDYT?

@pvalena
Copy link
Collaborator

pvalena commented Apr 20, 2021

One idea to consider for this: #701 (comment)

@jkonecny12
Copy link

Could you please explain how it should help here?

If I understand it correctly you would be able to do a provisioning before discover step by grouping plans? Am I correct?

@pvalena
Copy link
Collaborator

pvalena commented Apr 20, 2021

Well, it doesn't solve the tar creation (or unpacking) part, but that can be solved in a better way(#585).

If running provision+prepare step would be the only thing you need, you could run it manually like: tmt run provision prepare ... and then tmt run discover prepare -f execute. Theoretically there's nothing stopping you (I don't think the step order can be changed, but I may be mistaken). If you think that's what you need, simulate it and we could make it configurable. But I doubt that's what you actually need, as you, as you probably need to run prepare twice (second time for the tests dependencies instalation, as they're handled in prepare; therefore the -f =force), which is currently not possible (@psss can you clarify this please?).

Having the two plans- one that does the tests generation, and second one that does the actual tests execution (they would share the provision step and workdir). It would be like executing a second plan on the same tmt run ID, with leaving the host running (I'm not sure currently though, that discover is not destructive for the workdir- it probably is; which is something to be handled in that case).

Note that tere are several implementations proposed (at least from the tmt config POV), see others here: #694 (comment)

@pvalena
Copy link
Collaborator

pvalena commented Apr 20, 2021

One more thing to note is that the workdir is synced (currently still, AFAIK) back from guest, so you actually get everything to create back on host, which is what you need.


Ugly hack (bellow the line) would be to manually create / modify the generated tests.yaml (or using nested tmt!), which would be then synced back to host and executed.

The second (a bit less ugly) hack would be to simply run tmt nested on the guest (with execute --how local).

@jkonecny12
Copy link

jkonecny12 commented Apr 20, 2021

Thanks for pointing out the #694 but the ideal solution from @thrix doesn't solve our issue with tar ball preparation. You have to have these before discover step is taken. However, it's great for the problem they are trying to solve there.

@jkonecny12
Copy link

One more thing to note is that the workdir is synced (currently still, AFAIK) back from guest, so you actually get everything to create back on host, which is what you need.

Ugly hack (bellow the line) would be to manually create / modify the generated tests.yaml (or using nested tmt!), which would be then synced back to host and executed.

The second (a bit less ugly) hack would be to simply run tmt nested on the guest (with execute --how local).

The second hack could be used but I believe that this is issue significant enough to have it supported by TMT.

@lukaszachy
Copy link
Collaborator

From the security PoV even the git clone (CVE-2021-21300) can be a problem.
IMHO we should find a way how to jail tmt to workdir when it executes unsafe data (including git clone).

@pvalena
Copy link
Collaborator

pvalena commented Apr 22, 2021

In that case we probably want systemd-nspawn (mock uses it f.e.), but that would mean installing (mounting?) all dendencies to the "container".

@jkonecny12
Copy link

Couldn't you use podman run -v my_dir:/my_dir:O ? In other words to use overlay mount in the podman?

@jscotka
Copy link
Collaborator Author

jscotka commented Jan 3, 2023

as discussed on todays meeting. probably it would be easier, to have provisioned system, so that do these commands on target system, to improve security.

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

No branches or pull requests

5 participants