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

zypper: make XML parseable #20681

Closed
wants to merge 1 commit into from
Closed

zypper: make XML parseable #20681

wants to merge 1 commit into from

Conversation

@agx
Copy link
Contributor

@agx agx commented Jan 26, 2017

For the first package installed from a new repository zypper on (at
least) SLES12SP1 creates invalid XML for the GPG key information:

Repository: SLES12 Key Name: SuSE Package Signing Key Key Fingerprint: FEAB5025 39D846DB 2C0961CA 70AF9E81 39DB7C82 Key Created: Thu 31 Jan 2013 05:06:03 PM CET Key Expires: Mon 30 Jan 2017 05:06:03 PM CET (expires in 5 days) Rpm Name: gpg-pubkey-39db7c82-510a966b ... ... ...

The '<' and '>' in 'Key Name' are not escaped so initial package
installation from e.g. SLES12 Updates fails. Since the output is

  • always the first output after
  • starts with two spaces

we can filter it out and recover from the failure. In order to not
introduce other issues only reparse the document when we failed to parse
it in the first run.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

zypper

ANSIBLE VERSION

git devel HEAD

SUMMARY

see above

@ansibot
Copy link
Contributor

@ansibot ansibot commented Jan 26, 2017

@robinro
Copy link
Contributor

@robinro robinro commented Jan 26, 2017

Thanks for this PR @agx

In my opinion this is an upstream bug with zypper. Did you report it with Suse?
It's not too different from one I raised recently: https://bugzilla.opensuse.org/show_bug.cgi?id=1010712

I don't like the solution to mess with the output. I'd rather find a way to suppress the output. Can you get rid of the output by running before the zypper module:

zypper_repository:
  repo: '*'
  runrefresh: True
  auto_import_keys: True

If not maybe there is another way to make sure zypper doesn't show this?

In general we should now probably put a try/except around the xml parsing and at least show a better message.

dom = parseXML(stdout)
try:
dom = parseXML(stdout)
except Exception as e:
Copy link
Member

@mattclay mattclay Jan 26, 2017

Drop the as e since it's not used, and is not compatible with python 2.4.

@agx
Copy link
Contributor Author

@agx agx commented Jan 26, 2017

@robinro
Copy link
Contributor

@robinro robinro commented Jan 26, 2017

The bug report I mentioned above was fixed in less than a week. Broken xml is severe enough to have it fixed upstream.

Does the explicit key fetch I mentioned help to work around the issue? If so I'd rather not merge this.

For the first package installed from a new repository zypper on (at
least) SLES12SP1 creates invalid XML for the GPG key information:

  <?xml version='1.0'?>
  <stream>
    Repository:       SLES12
    Key Name:         SuSE Package Signing Key <build@suse.de>
    Key Fingerprint:  FEAB5025 39D846DB 2C0961CA 70AF9E81 39DB7C82
    Key Created:      Thu 31 Jan 2013 05:06:03 PM CET
    Key Expires:      Mon 30 Jan 2017 05:06:03 PM CET (expires in 5 days)
    Rpm Name:         gpg-pubkey-39db7c82-510a966b
  <install-summary download-size="1104448" space-usage-diff="264635">
  ...
  </install-summary>
  <prompt id="0">
  ...
  </prompt>
  ...
  </stream>

The '<' and '>' in 'Key Name' are not escaped so initial package
installation from e.g. SLES12 Updates fails. Since the output is

  - always the first output after <stream>
  - starts with two spaces

we can filter it out and recover from the failure. In order to not
introduce other issues only reparse the document when we failed to parse
it in the first run.
@agx
Copy link
Contributor Author

@agx agx commented Jan 27, 2017

@robinro
Copy link
Contributor

@robinro robinro commented Feb 11, 2017

As discussed before I'd prefer to fix this upstream instead of adding this hack.
Also as far as I understand the issue, one can work around the specific issue by importing the gpg key before, see #20681 (comment)
close_me

To have nicer error output of the issue I opened #21281
If the problem can not be worked around and won't be fixes upsteam I'd be happy to reopen this one again.

@ansibot ansibot closed this Feb 11, 2017
@mlandres
Copy link

@mlandres mlandres commented Feb 14, 2017

JFYI: Importing the gpg key before does not always help. Zypper will also display the key info if a key is about to expire soon.
Fixed:
zypper 1.13.16 (SLE-12-SP2/Leap42.2)
zypper 1.12.48 (SLE-12-SP1/Leap42.1)
zypper 1.11.61 (SLE-12)

@robinro
Copy link
Contributor

@robinro robinro commented Feb 14, 2017

@mlandres
Thanks for the fix!

Do you have a suggestion how to improve the ansible<->zypper communication in the long term? Is using the xml interface a good idea? It seems somewhat buggy and unused, but maybe most issues are fixed now?
I thought about talking to libzypp directly, but there is no python interface and C/Ruby interfaces are more complex to implement than shell/xml, especially when supporting as many versions as ansible and suse do.

@mlandres
Copy link

@mlandres mlandres commented Feb 14, 2017

@robinro Don't know if most issues are fixed, but we'll continue to fix the issues arising with the XML interface. Since ansible and SALT are using it, things seem to improve a bit.

There is a libzypp-bindings package offering some python/ruby interface. It uses swig to directly wrap the libzypp classes, or at least some of them. The problem is that it's incomplete, quite lowlevel, sometimes broken and more or less unmaintained.
I'd really like to offer some bindings maintained together with libzypp in the future, but currently we have no resources for this. So XML is probably the best choice.

@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants