[
https://issues.apache.org/jira/browse/HDDS-6365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17504996#comment-17504996
]
Attila Doroszlai commented on HDDS-6365:
----------------------------------------
bq. I do not see any mention of kerberosID in the current
./hadoop-hdds/interface-client/src/main/resources/proto.lock. So maybe changing
this field name wouldn't trigger the protolock check failure after all.
That's because it is in
{{hadoop-ozone/interface-client/src/main/resources/proto.lock}} ({{ozone}}, not
{{hdds}}).
{code}
hadoop-ozone/interface-client/src/main/resources/proto.lock
3894: "name": "kerberosID",
3909: "name": "kerberosID",
3939: "name": "kerberosID",
3949: "name": "kerberosID",
{code}
And compatibility check does fail if these are renamed:
{code}
[INFO] --- proto-backwards-compatibility:1.0.7:backwards-compatibility-check
(default) @ ozone-interface-client ---
[INFO] protolock cmd line:
hadoop-ozone/interface-client/target/protolock-bin/protolock status
--lockdir=hadoop-ozone/interface-client/target/classes
--protoroot=hadoop-ozone/interface-client/target/classes
[INFO] CONFLICT: "GetS3SecretRequest" field: "accessId" ID: 1 has an updated
name, previously "kerberosID" [OmClientProtocol.proto]
[INFO] CONFLICT: "GetS3SecretRequest" field: "kerberosID" has been removed, but
is not reserved [OmClientProtocol.proto]
[INFO] CONFLICT: "RevokeS3SecretRequest" field: "accessId" ID: 1 has an updated
name, previously "kerberosID" [OmClientProtocol.proto]
[INFO] CONFLICT: "RevokeS3SecretRequest" field: "kerberosID" has been removed,
but is not reserved [OmClientProtocol.proto]
[INFO] CONFLICT: "S3Secret" field: "accessId" ID: 1 has an updated name,
previously "kerberosID" [OmClientProtocol.proto]
[INFO] CONFLICT: "S3Secret" field: "kerberosID" has been removed, but is not
reserved [OmClientProtocol.proto]
[INFO] CONFLICT: "UpdateGetS3SecretRequest" field: "accessId" ID: 1 has an
updated name, previously "kerberosID" [OmClientProtocol.proto]
[INFO] CONFLICT: "UpdateGetS3SecretRequest" field: "kerberosID" has been
removed, but is not reserved [OmClientProtocol.proto]
...
[ERROR] Failed to execute goal
com.salesforce.servicelibs:proto-backwards-compatibility:1.0.7:backwards-compatibility-check
(default) on project ozone-interface-client: Backwards compatibility check
failed! You can override this by specifying allowBreakingChanges=true
{code}
> Relax protolock rule to allow changing field names
> --------------------------------------------------
>
> Key: HDDS-6365
> URL: https://issues.apache.org/jira/browse/HDDS-6365
> Project: Apache Ozone
> Issue Type: Improvement
> Components: build
> Reporter: Siyao Meng
> Assignee: Siyao Meng
> Priority: Major
> Labels: pull-request-available
>
> h2. Motivation
> The motivation behind this is that currently in the master branch we use
> {{kerberosID}} as the protobuf message field name while it really means
> {{accessId}}. For example here:
> https://github.com/apache/ozone/blob/master/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto#L1323
> It just so happens to be the case that before Multi-Tenancy (HDDS-4944) is
> implemented, kerberosID is equivalent to accessId in the context of Ozone S3
> access. But with Multi-Tenancy feature this is no longer the case -- accessId
> does not equal to kerberosID any more.
> And it could be confusing for other developers to work on S3 / Multi-Tenancy
> related stuff in the future. e.g. those {{getKerberosID}} calls really means
> {{getAccessId}}, and {{setKerberosID}} really is {{setAccessId}}. Filed
> HDDS-6339.
> h2. Status Quo
> Right now {{proto-backwards-compatibility}} uses default
> [protolock|https://github.com/nilslice/protolock#usage] command line
> argument, effectively enforcing strict mode for the
> [rules|https://github.com/nilslice/protolock#rules-enforced], which doesn't
> allow changing existing protobuf field names:
> {code:xml|title=https://github.com/apache/ozone/blob/68c5ac5df4fbc0edd7114394112fa696a3dc9229/pom.xml#L1619-L1633}
> <groupId>com.salesforce.servicelibs</groupId>
> <artifactId>proto-backwards-compatibility</artifactId>
> <version>${proto-backwards-compatibility.version}</version>
> <configuration>
> <protoSourceRoot>${basedir}/target/classes</protoSourceRoot>
> </configuration>
> {code}
> Changing the field name itself does not break wire compatiblity (field type
> and field number are still the same), unless that protobuf message is decoded
> into JSON at some point (which could use the field name as key). Ref:
> https://stackoverflow.com/a/45431953
> h2. Proposal
> Add {{<options>--strict false</options>}} in {{pom.xml}} so that strict mode
> is off, which as a side affect also turns off two other rules currently
> enforced according to [protolock
> readme|https://github.com/nilslice/protolock#rules-enforced]. I'm not sure if
> protolock provides a way for more granular control of which exact rule to
> turn off.
> {code}
> No Removing Reserved Fields
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any reserved field has been removed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
> {code}
> No Changing Field Names
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any message's previous fields have been renamed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
> {code}
> No Removing RPCs
> Compares the current vs. updated Protolock definitions and will return a list
> of warnings if any RPCs provided by a Service have been removed.
> Note: This rule is not enforced when strict mode is disabled.
> {code}
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
