fix_format doesn't uniquely identify absolute protobuf message names
- Dominant language
- C++
- Stars
- 28.9k
- Forks
- 5.6k
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 430
Description
We currently have the following structure inside of [`envoy.extensions.transport_sockets.tls.v3.CommonTlsContext`](https://github.com/envoyproxy/envoy/blob/7da9989039cc7fbbe08835e2c00f813aedb24e03/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L129):
- There is a [`CertificateProviderInstance`](https://github.com/envoyproxy/envoy/blob/7da9989039cc7fbbe08835e2c00f813aedb24e03/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L154) message.
- There is a field called [`tls_certificate_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/7da9989039cc7fbbe08835e2c00f813aedb24e03/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L234) inside of `CommonTlsContext` that use this type.
In #17201, I am moving the `CertificateProviderInstance` out of `CommonTlsContext`. In order to not break API compatibility, I need to leave the existing `CertificateProviderInstance` message and the field that uses it intact but deprecated, copy the message to the new location, and add new fields that use the message in the new location.
So I created the new message and added the following new field in `CommonTlsContext`:
```
.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance tls_certificate_provider_instance = 15;
```
That uses the full path of the proto to differentiate between the new version of the `CertificateProviderPluginInstance` message that exists outside of the `CommonTlsContext` proto from the old version that exists inside of `CommonTlsContext`. However, fix_format changed it to use the latter instead of the former:
```
CertificateProviderInstance tls_certificate_provider_instance = 15;
```
It seems that fix_format can't tell that `CertificateProviderInstance` exists both inside and outside of `CommonTlsContext` and that I wanted to refer to the one that is outside.
For now, I've worked around this by simply using a different name for the new message (`CertificateProviderPluginInstance` instead of `CertificateProviderInstance`). But I think we should fix fix_format to not require that.
CC @htuch
Contributor guide
Assessment
This issue has not been assessed yet.