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

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

Open
happz opened this issue Nov 15, 2022 · 5 comments
Open

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

happz opened this issue Nov 15, 2022 · 5 comments
Assignees
Labels
command | export The export command

Comments

@happz
Copy link
Collaborator

happz commented Nov 15, 2022

$ 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.

@happz happz self-assigned this Nov 15, 2022
@psss
Copy link
Collaborator

psss commented Dec 6, 2022

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

happz commented Dec 6, 2022

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

psss commented Dec 6, 2022

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

Ack.

@psss psss added this to the 1.20 milestone Dec 6, 2022
@psss psss added the command | export The export command label Dec 6, 2022
@happz
Copy link
Collaborator Author

happz commented Dec 6, 2022

@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 #1729, later I'd add a more mature fix & close this issue.

@psss
Copy link
Collaborator

psss commented Dec 6, 2022

Understood, ok.

@psss psss removed this from the 1.20 milestone Dec 6, 2022
happz added a commit that referenced this issue Dec 6, 2022
Fields starting with underscore, `_`, are considered "private" and
should not be exported.

This is a "dirty" workaround to fix #1729, a proper fix would probably
turn the table around, and instead of hiding some fields it would export
only fields marked for export. That should be implemented later for #1688.

Fixes #1729.
psss pushed a commit that referenced this issue Dec 6, 2022
Fields starting with underscore, `_`, are considered "private" and
should not be exported.

This is a "dirty" workaround to fix #1729, a proper fix would probably
turn the table around, and instead of hiding some fields it would export
only fields marked for export. That should be implemented later for #1688.

Fixes #1729.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command | export The export command
Projects
None yet
Development

No branches or pull requests

2 participants