Summary

Promote some mandatory properties from cert storage backends to base.

Review Request #14898 — Created March 12, 2026 and updated April 1, 2026, 10:35 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

BaseStoredCertificate.hostname
BaseStoredCertificate.port
BaseStoredCertificateBundle.name
BaseStoredCertificateFingerprints.hostname
BaseStoredCertificateFingerprints.port

Testing Done

Unit tests pass.

Commits

Summary	ID
Promote some mandatory properties from cert storage backends to base. There are some important variables in the SSL/TLS cert storage backends that are currently locked into the file storage backend. This promotes the following from that backend into the base classes and adds validation during construction: * `BaseStoredCertificate.hostname` * `BaseStoredCertificate.port` * `BaseStoredCertificateBundle.name` * `BaseStoredCertificateFingerprints.hostname` * `BaseStoredCertificateFingerprints.port`	c56dfaa8f3b49c02237c23f7e12d4a9ae296fb5f

Summary

Promote some mandatory properties from cert storage backends to base.

There are some important variables in the SSL/TLS cert storage backends that are currently locked into the file storage backend. This promotes the following from that backend into the base classes and adds validation during construction: * `BaseStoredCertificate.hostname` * `BaseStoredCertificate.port` * `BaseStoredCertificateBundle.name` * `BaseStoredCertificateFingerprints.hostname` * `BaseStoredCertificateFingerprints.port`

c56dfaa8f3b49c02237c23f7e12d4a9ae296fb5f

Issues

Description	From	Last Updated
I think we can get rid of the "These will be mandatory in Review Board 9", they're only mandatory if …	maubin	April 1, 2026, 9:51 p.m.
Copy/pasteo: hostname -> port	david	April 1, 2026, 9:53 p.m.
Can you add a Raises section in the docs for these.	maubin	April 1, 2026, 9:52 p.m.
Do we need to go through a release cycle where these are optional and say "These will be mandatory in …	maubin	April 1, 2026, 9:53 p.m.
Copy/pasteo: hostname -> port	david	April 1, 2026, 9:53 p.m.
There are double spaces on these (space at the end of one line and the beginning of the next)	david	April 1, 2026, 9:54 p.m.
This should end with )>	david	April 1, 2026, 9:54 p.m.
This test says "missing hostname" but your certificate has a hostname, so it's testing a missing port. We should probably …	david	April 1, 2026, 10:03 p.m.

flake8 passed.

JSHint passed.

reviewboard/certs/storage/base.py (Diff revision 1)

The issue has been resolved. Show all issues

I think we can get rid of the "These will be mandatory in Review Board 9", they're only mandatory if certificate is not set. And if we get rid of that we can merge this with the above bullet point. I guess you can say "These will be mandatory if certificate is not provided".

reviewboard/certs/storage/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you add a Raises section in the docs for these.
```

reviewboard/certs/storage/base.py (Diff revision 1)

The issue has been dropped. Show all issues

Do we need to go through a release cycle where these are optional and say "These will be mandatory in Review Board X" before we make them mandatory?

chipx86 April 1, 2026, 10:03 p.m.

Because none of this was used in prior releases, we're going to forgo the deprecation phase.

reviewboard/certs/storage/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Copy/pasteo: hostname -> port
```
reviewboard/certs/storage/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Copy/pasteo: hostname -> port
```

reviewboard/certs/storage/file_storage.py (Diff revision 1)

The issue has been resolved. Show all issues

There are double spaces on these (space at the end of one line and the beginning of the next)

reviewboard/certs/storage/file_storage.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should end with )>
```

reviewboard/certs/tests/test_file_storage_backend.py (Diff revision 1)

The issue has been resolved. Show all issues

This test says "missing hostname" but your certificate has a hostname, so it's testing a missing port. We should probably fix this one and introduce a second test case for a missing port.

Change Summary:


Updated versions for RB 8.
Fixed some unit test names.
Removed the incorrect note about some arguments being mandatory.
FIxed a unit test for FileStoredCertificate.__init__ testing port instead of hostname, and added a test for port.

Commits:

	Summary	ID
	Promote some mandatory properties from cert storage backends to base. There are some important variables in the SSL/TLS cert storage backends that are currently locked into the file storage backend. This promotes the following from that backend into the base classes and adds validation during construction: * `BaseStoredCertificate.hostname` * `BaseStoredCertificate.port` * `BaseStoredCertificateBundle.name` * `BaseStoredCertificateFingerprints.hostname` * `BaseStoredCertificateFingerprints.port`	58cfc367f643a0448defcfbe8aa0157d64f1598d
	Promote some mandatory properties from cert storage backends to base. There are some important variables in the SSL/TLS cert storage backends that are currently locked into the file storage backend. This promotes the following from that backend into the base classes and adds validation during construction: * `BaseStoredCertificate.hostname` * `BaseStoredCertificate.port` * `BaseStoredCertificateBundle.name` * `BaseStoredCertificateFingerprints.hostname` * `BaseStoredCertificateFingerprints.port`	c56dfaa8f3b49c02237c23f7e12d4a9ae296fb5f

Diff:

Revision 2 (+936 -562)

Show changes

	reviewboard/certs/storage/base.py
	reviewboard/certs/storage/file_storage.py
	reviewboard/certs/tests/test_file_storage_backend.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```