ENG-8624 fix operating_system_version/operating_system mismatch

[kqdeng]

Signed-off-by: Kelly Deng [email protected]

Description

This PR fixes the mismatch in field names operating_system_version vs operating_system.

Why is this needed

This bug mainly affects the users who needs both the packet.pb.go and the EC2 endpoint. Because the EC2 endpoint in Hegel uses the field name operating_system to be backwards compatible with Kant, users using the Metadata object defined in packet.pb.go will face a problem with parsing the operating system field since it was defined as operating_system_version there.
The reason for the mismatch is because in the "old" cacher mode, operating_system_version, but Kant uses operating_system (which we have to be backwards compatible with) so we decided move away from operating_system_version and go with operating_system for "tink mode".
A user could have worked around this by specifying two fields operating_system and operating_system_version with the same value, inside the same piece of hardware, but this introduces some redundancy and just isn't ideal.

Related PR: tinkerbell/smee#104

Fixes: #

How Has This Been Tested?

To be manually tested

How are existing users impacted? What migration steps/scripts do we need?

Existing users who import the packet.pb.go will have to go get latest master of the tink repo.

Checklist:

I have:

• updated the documentation and/or roadmap (if required)
• added unit or e2e tests
• provided instructions on how to upgrade

[codecov]

Codecov Report

Merging #275 (df9951b) into master (86057d3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   24.74%   24.74%           
=======================================
  Files          14       14           
  Lines        1277     1277           
=======================================
  Hits          316      316           
  Misses        940      940           
  Partials       21       21           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86057d3...df9951b. Read the comment docs.

[gianarb]

Is this a bc break? How does it impact users? Can we add a bit more context to this pull request in the description?
Thanks

[displague]

@kdeng3849 Can you rebase this and update with the latest protos/protoc.sh (presumably this should result in a smaller diff, especially if the containerized protoc is used).

To @gianarb's point, can we offer both operating_system and operating_system_version to avoid a BC break? Does this make sense in the long-run? (operating_system=debian, operating_system_version=9.0)

[kqdeng]

@displague I'm not sure if that makes sense because the operating_system/operating_system_version field is actually an object (not string and float, unless I misunderstood you there).

I guess we could offer both for backward compatibility, but I don't think many people were using the code generated from the packet proto to begin with? And I also think operating_system makes more sense compared to operating_system_version since with version in the name people might think it's actually referring to a version number, when it's not. Just feels unintuitive to me.

[mmlb]

This isn't really a bc-break as packet.proto is not supported in the oss project. Its being used by one internal client that is currently broken and needs this PR so it works. Either way, packet.proto should be replaced and actually be usable by tinkerbell/proposals#25