Is tmt plan import doing the actual import too early?

kkaarreell commented 3 months ago

I was trying to add a condition around a test plan import but it didnt' work as expected. E.g. this example with bogus url produces git clone error despite the plan being disabled.

/keylime-e2e:
  plan:
    import:
      url: hhttps://github.com/RedHat-SP-Security/keylime-plans.git
      name: /generic/minimal-e2e
  enabled: 0

$ tmt -c distro=rhel-10 run discover 
/var/tmp/tmt/run-015
    warn: /ci/keylime-e2e:enabled - 0 is not of type 'boolean'
fail: Command 'git clone hhttps://github.com/RedHat-SP-Security/keylime-plans.git /var/tmp/tmt/run-015/import/ci/keylime-e2e' returned 128.

Maybe the above is expected. I basically wanted to avoid fetching an internal URL for distros like Fedora or CS. For now I ended up with this which seems to work but looks unnecessarily complicated

/keylime-e2e:
  enabled: 0

  adjust:
   - when: distro == rhel
     enabled: 1
     plan:
      import:
        url: hhttps://github.com/RedHat-SP-Security/keylime-plans.git
        name: /generic/minimal-e2e

I am wondering if there is effectively any difference between these two approaches when the 1st one is doing the import early while the later one late. I can imagine that the imported plan may have also some adjust rules impacting plan status.

LecrisUT commented 3 months ago

enabled is completely ignored by plan.import. I think there was a discussion on that when it was initially implemented.

kkaarreell commented 3 months ago

No, it is supported. https://github.com/teemtee/tmt/commit/0bbd0ec25082e473643684e000685b01ba659285 But the problem here is that git clone of the imported plan is happening before the plan is disabled.

psss commented 3 months ago

For reference, this is how the enabled key is handled during import:

The imported plan can also be altered using the enabled key. If the local plan is enabled, it will follow the status of the remote plan – whether it’s enabled or disabled. If the local plan is disabled, the remote plan will also be disabled. Adjust rules are respected during this process.

psss commented 3 months ago

I can imagine that the imported plan may have also some adjust rules impacting plan status.

Yes, adjust rules from the remote plans are applied. Even if the plan is disabled, we're always trying to fetch the details, unless for example --shallow option is used for tmt plan ls and friends.

Not sure how to approach this, as for tmt plan show even disabled plan should be fetched (always), but for the tmt run use case, probably all plans with enabled: false should be ignored right away.

Perhaps, for the tmt run case we could do a shallow check for existing plans first? And then fetch all details for plans which are enabled?

teemtee / tmt

Is tmt plan import doing the actual import too early? #2968