Fix validating and trusting Perforce repositories with Mirror Paths.

Review Request #11007 — Created May 4, 2020 and submitted

Information

Review Board
release-3.0.x

Reviewers

Perforce repositories work differently from all other types of
repositories. Instead of all communication happening over the Path, it
happens over the Mirror Path. This is done for very ancient historical
reasons, centered around a very early need for tools like Review Board
using a read-only Perforce mirror that was separate from the main
read-write repository that developers had been using. This inversed
logic is sadly probably here to stay long-term.

The problem is, the repository form didn't know about Perforce's needs
here, and was checking only the Path field, not Mirror Path, when
validating a repository. This was generally fine for most usage, but if
using an SSL-backed repository in Mirror Path that had to be explicitly
trusted, Review Board would never have an opportunity to validate this
and present it to the user. The only way it'd work is to have the SSL
repository in Path, and a non-SSL in Mirror Path, at which point the
communication wouldn't be encrypted.

This change fixes this up by adding a new SCMTool.prefers_mirror_path
flag, which instructs the form to check using the Mirror Path if set.
Only Perforce has this enabled, and the documentation discourages others
from ever setting it.

In time, we can clean some of this up at a presentational level by
creating a custom subform and making it clear which is used for which.
This change does take a step toward this by adding new help text for
Mirror Path that makes it clear it takes precedence for communication.

Unit tests pass.

Manually tested adding a local repository and verifying the paths it
was using.

Summary ID
Fix validating and trusting Perforce repositories with Mirror Paths.
Perforce repositories work differently from all other types of repositories. Instead of all communication happening over the Path, it happens over the Mirror Path. This is done for very ancient historical reasons, centered around a very early need for tools like Review Board using a read-only Perforce mirror that was separate from the main read-write repository that developers had been using. This inversed logic is sadly probably here to stay long-term. The problem is, the repository form didn't know about Perforce's needs here, and was checking only the Path field, not Mirror Path, when validating a repository. This was generally fine for most usage, but if using an SSL-backed repository in Mirror Path that had to be explicitly trusted, Review Board would never have an opportunity to validate this and present it to the user. The only way it'd work is to have the SSL repository in Path, and a non-SSL in Mirror Path, at which point the communication wouldn't be encrypted. This change fixes this up by adding a new `SCMTool.prefers_mirror_path` flag, which instructs the form to check using the Mirror Path if set. Only Perforce has this enabled, and the documentation discourages others from ever setting it. In time, we can clean some of this up at a presentational level by creating a custom subform and making it clear which is used for which. This change does take a step toward this by adding new help text for Mirror Path that makes it clear it takes precedence for communication.
cec4eea031988bc5d4e4452ac32e1a5a1f40c136
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (74b3a44)