Skip to content

Wip fix gitaly client storage settings gitaly address method

What does this MR do?

The Gitlab::GitalyClient::StorageSettings#gitaly_address currently fails, because the GitLab configurations in both the production and staging environments defines the gitaly address entry for each node's storage setting using a string key, but the method actually tries to fetch the value based on a symbolic key.

Interestingly enough, the config/gitlab.yml.example file uses a symbolic key.

This implies that there was a mistake made when the gitaly address for each node's storage settings were added. Both fortunately and not, the Gitlab::GitalyClient::StorageSettings#gitaly_address is actually never invoked anywhere in the codebase, and so this method could be considered to be effectively "dead code". However, it is certainly convenient to be able to have access to the gitaly_address in side-car tooling, and in the rails console.

This fix simply introduces a secondary fetch using a string key, as a fall-back for the case in which a string key appears in the storage settings of gitlab/config.rb.

Screenshots

No relevant screenshots -- not a UI change.

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

  • What risks does this change pose? How might it affect the quality/performance of the product?

This change poses little risk, especially considering that the altered code lines are not explicitly invoked by the program.

  • What additional test coverage or changes to tests will be needed?

Added four additional spec tests to cover the current failure mode and additional behavioral expectations for the new functionality.

  • Will it require cross-browser testing?

No -- not a UI change.

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Nels Nelson

Merge request reports