Skip to content

refactor: rewrite cert updater script to Python#605

Draft
okurz wants to merge 2 commits into
os-autoinst:masterfrom
okurz:feature/cert_update_python
Draft

refactor: rewrite cert updater script to Python#605
okurz wants to merge 2 commits into
os-autoinst:masterfrom
okurz:feature/cert_update_python

Conversation

@okurz

@okurz okurz commented Jun 16, 2026

Copy link
Copy Markdown
Member

Motivation:
Provide a robust command-line interface with proper --help documentation for
the certificate updating script.

Design Choices:
Port the bash script to Python using typer for argument parsing. Relocate
installation and usage documentation from README.md to the typer --help
block. Add unit tests with 100% statement and branch coverage.

Benefits:
Easier maintenance, better command-line documentation, and rigorous test
coverage.

After:

Martchus and others added 2 commits June 16, 2026 14:11
These certificates are not published somewhere so we need to use `osc` to
download them.

This change adds a systemd unit and timer that are supposed to be used in
user mode. It also contains an install target for the systemd units. It is
not suited for packaging and mainly to make it clear where the unit should
be placed to use them locally. It is also useful for local testing:
```
$ make install-systemd-local DESTDIR=/tmp/install
$ cp -vR /tmp/install/etc/systemd/user/ ~/.config/systemd/
$ systemctl --user daemon-reload
```

Then the timer unit can be tested via:
```
$ systemctl --user start \
  os-autoinst-scripts-update-factory-staging-cert.timer
```

The service units can be tested via:
```
$ systemctl --user start \
  os-autoinst-scripts-update-cert-from-obs@factory-staging.service
$ journalctl --user -u \
  os-autoinst-scripts-update-cert-from-obs@factory-staging.service
Starting Update a certificate from OBS...
certificate written to …/factory/other/fixed/openSUSE-Factory-Staging.crt
Finished Update a certificate from OBS.
```

```
$ systemctl --user start \
  os-autoinst-scripts-update-cert-from-obs@suse-unsupported.service
$ journalctl --user -u \
  os-autoinst-scripts-update-cert-from-obs@suse-unsupported.service
Starting Update a certificate from OBS...
certificate written to …/factory/other/fixed/suse-unsupported.crt
Finished Update a certificate from OBS.
```

This works on my system. On the production hosts we should probably create
a dedicated user account with the required osc config and write
permissions in the assets directory.

Related ticket: https://progress.opensuse.org/issues/200075
Motivation:
Provide a robust command-line interface with proper --help documentation for
the certificate updating script.

Design Choices:
Port the bash script to Python using typer for argument parsing. Relocate
installation and usage documentation from README.md to the typer --help
block. Add unit tests with 100% statement and branch coverage.

Benefits:
Easier maintenance, better command-line documentation, and rigorous test
coverage.

[Service]
Type=oneshot
ExecStart=/usr/bin/bash /opt/os-autoinst-scripts/openqa-update-staging-cert /opt/os-autoinst-scripts/config/obs-certs/%i.conf

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue(blocking):

Suggested change
ExecStart=/usr/bin/bash /opt/os-autoinst-scripts/openqa-update-staging-cert /opt/os-autoinst-scripts/config/obs-certs/%i.conf
ExecStart=/usr/bin/bash /opt/os-autoinst-scripts/openqa-update-staging-cert-from-obs /opt/os-autoinst-scripts/config/obs-certs/%i.conf

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fixed in my version so this PR is probably not based on the current version of my PR.

Comment thread systemd/user/os-autoinst-scripts-update-cert-from-obs@.service
Comment thread systemd/user/os-autoinst-scripts-update-cert-from-obs@.service
@Martchus

Copy link
Copy Markdown
Contributor

I don't think the additional 400 lines and additional dependencies are justified here. Let's merge my PR first and see how it works in production.

@Martchus

Copy link
Copy Markdown
Contributor

@Wabri I implemented your suggestions in #603.

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

Successfully merging this pull request may close these issues.

3 participants