teemtee / tmt

Test Management Tool
MIT License
86 stars 129 forks source link

Avoid leaking non-key members into keys()/items()/to_dict()/... output #1688

Open happz opened 2 years ago

happz commented 2 years ago
$ tmt tests export --format yaml /tests/artemis/scenarios/tests/hw/arch/aarch64
  - name: /tests/artemis/scenarios/tests/hw/arch/aarch64
    summary:
    description:
    enabled: true
    order: 50
    link: []
    id:
    tag:
      - complete
    tier:
    contact: []
    component: []
    test: ./test.sh
    path: /tests/artemis/scenarios/tests/hw/arch
    framework: shell
    manual: false
    require: []
    recommend: []
    environment:
        EXPECTED: aarch64
    duration: 5m
    result: respect
    returncode:
    real_duration:
    _reboot_count: 0

$ tmt plans export --format yaml /tests/artemis/scenarios/plans/openstack/hw/virtualization/is-virtualized/yes/x86_64
  - name: /tests/artemis/scenarios/plans/openstack/hw/virtualization/is-virtualized/yes/x86_64
    summary:
    description:
    enabled: true
    order: 50
    link: []
    id:
    tag: []
    tier:
    context: {}
    gate: []
    _imported_plan:
    _original_plan:
    _remote_plan_fmf_id:
    discover:
        how: fmf
        test:
          - hw/virtualization/is-virtualized/yes
        name: default-0
    provision:
        how: artemis
        api-url: ${ARTEMIS_API_URL}
        api-version: ${ARTEMIS_API_VERSION}
        image: ${ARTEMIS_COMPOSE}
        keyname: ${ARTEMIS_KEYNAME}
        pool: ${ARTEMIS_OPENSTACK_POOLNAME}
        hardware:
            virtualization:
                is-virtualized: true
        arch: x86_64
        name: default-0
    execute:
        how: tmt
        name: default-0

Note the keys beginning with underscore, _imported_plan, _reboot_count, and so on - these do not have any place to be in the output of keys() and & co, as they are not keys recognized by tmt specification, but implementation details. We need a way to avoid exposing them.

psss commented 1 year ago

Note the keys beginning with underscore, _imported_plan, _reboot_count, and so on - these do not have any place to be in the output of keys() and & co, as they are not keys recognized by tmt specification, but implementation details. We need a way to avoid exposing them.

Good point. As those internal keys are not part of the specification we could possibly just ignore them? At least during the export.

happz commented 1 year ago

Note the keys beginning with underscore, _imported_plan, _reboot_count, and so on - these do not have any place to be in the output of keys() and & co, as they are not keys recognized by tmt specification, but implementation details. We need a way to avoid exposing them.

Good point. As those internal keys are not part of the specification we could possibly just ignore them? At least during the export.

Not a good idea in the long run, as some names not worthy of exporting can easily not start with an underscore, but I agree it's the best we can do to patch it for 1.20/quickly. The proper solution would land later.

psss commented 1 year ago

I agree it's the best we can do to patch it for 1.20/quickly. The proper solution would land later.

Ack.

happz commented 1 year ago

@psss if you won't mind, I'd keep this issue open, i.e. not something to include in 1.20 - a quick & dirty fix to resolve https://github.com/teemtee/tmt/issues/1729, later I'd add a more mature fix & close this issue.

psss commented 1 year ago

Understood, ok.