Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(kuma-cp) upgrade Protocol Buffers #2244

Merged
merged 15 commits into from
Jul 2, 2021
Merged

chore(kuma-cp) upgrade Protocol Buffers #2244

merged 15 commits into from
Jul 2, 2021

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

Update Protocol Buffers dependencies so we can update other stuff.

Documentation

  • No docs

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

@jakubdyszkiewicz
chore(kuma-cp) upgrade protobufs
39ff043 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
fix imports and tabs
b4efdbe 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #2244 (39ff043) into master (8b13853) will increase coverage by 0.38%.
The diff coverage is 59.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2244      +/-   ##
==========================================
+ Coverage   51.76%   52.14%   +0.38%     
==========================================
  Files         909      912       +3     
  Lines       41004    41011       +7     
==========================================
+ Hits        21224    21386     +162     
+ Misses      17743    17573     -170     
- Partials     2037     2052      +15
Impacted Files Coverage Δ
api/mesh/options.pb.go 29.41% <ø> (ø)
api/mesh/v1alpha1/circuit_breaker.pb.go 46.09% <ø> (ø)
api/mesh/v1alpha1/dataplane.pb.go 36.83% <ø> (ø)
api/mesh/v1alpha1/dataplane_insight.pb.go 42.19% <ø> (ø)
api/mesh/v1alpha1/dataplane_overview.pb.go 47.36% <ø> (ø)
api/mesh/v1alpha1/externalservice.pb.go 52.41% <ø> (+4.03%) ⬆️
api/mesh/v1alpha1/fault_injection.pb.go 47.25% <ø> (ø)
api/mesh/v1alpha1/health_check.pb.go 31.87% <ø> (+1.99%) ⬆️
api/mesh/v1alpha1/kds.pb.go 29.25% <ø> (ø)
api/mesh/v1alpha1/mesh.pb.go 33.40% <ø> (+1.01%) ⬆️
... and 77 more

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 8b13853...39ff043. Read the comment docs.

@jakubdyszkiewicz jakubdyszkiewicz marked this pull request as ready for review June 29, 2021 14:56
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner June 29, 2021 14:56
bartsmykla
bartsmykla approved these changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@bartsmykla bartsmykla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

jpeach
jpeach reviewed Jun 29, 2021
View reviewed changes
pkg/kds/util/resource.go Outdated Show resolved Hide resolved
jpeach
jpeach reviewed Jun 29, 2021
View reviewed changes
api/Makefile Show resolved Hide resolved
jpeach
jpeach reviewed Jun 29, 2021
View reviewed changes
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about the intended scope of these changes. Is it just upgrading github.com/golang/protobuf (in which case I probably expected to see fewer changes), or is it converting Kuma internals over to google.golang.org/protobuf (in which case maybe we should consider more changes to really isolate the old package).

@jakubdyszkiewicz
Merge remote-tracking branch 'origin/master' into chore/upgrade-proto
0433436 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
update duration imports
5c511cf 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
upgrade wrappers
31b108b 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
migrate structpb
493cde5 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
migrate types for generated protos
3bb7d00 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
update deperecated types in api module
567e4c5 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
make check
5f306fb 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
introduce marshal util
bfdeedb 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
make check
ee987f5 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
Copy link
Contributor Author

The goal of PR is to upgrade Protobuf version. The problem is that just upgrading it results in many linter errors of deprecation, that's why I had to fix it by changing imports.

I fixed many imports to a new one, but not all of this. The rest is not a simple change.
I'd rather merge this PR and gradually upgrade more because it will be hard to maintain this PR with so many changes.

@jpeach
Copy link
Contributor

jpeach commented Jul 2, 2021

I fixed many imports to a new one, but not all of this. The rest is not a simple change.
I'd rather merge this PR and gradually upgrade more because it will be hard to maintain this PR with so many changes.

Yup I agree that makes sense. Let's get a github issue open with a list of the remaining work while it's fresh in your mind :)

jpeach
jpeach approved these changes Jul 2, 2021
View reviewed changes
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good improvement to me. Let's merge once integration tests are passing.

Note we still ought to update ./pkg/test/xds/client/go.mod, but we can do that inn a separate PR.

@jakubdyszkiewicz
spaces
6d10da1 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz
make check
003a7bb 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
@jakubdyszkiewicz jakubdyszkiewicz merged commit 19ec14b into master Jul 2, 2021
@jakubdyszkiewicz jakubdyszkiewicz deleted the chore/upgrade-proto branch July 2, 2021 12:38
mergify bot pushed a commit that referenced this pull request Jul 2, 2021
@jakubdyszkiewicz
chore(kuma-cp) upgrade Protocol Buffers (#2244)
e6f0535 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
(cherry picked from commit 19ec14b)

# Conflicts:
#	api/mesh/options.pb.go
#	api/mesh/v1alpha1/circuit_breaker.pb.go
#	api/mesh/v1alpha1/dataplane.pb.go
#	api/mesh/v1alpha1/dataplane_insight.pb.go
#	api/mesh/v1alpha1/dataplane_overview.pb.go
#	api/mesh/v1alpha1/externalservice.pb.go
#	api/mesh/v1alpha1/fault_injection.pb.go
#	api/mesh/v1alpha1/mesh.pb.go
#	api/mesh/v1alpha1/mesh_insight.pb.go
#	api/mesh/v1alpha1/proxy_template.pb.go
#	api/mesh/v1alpha1/rate_limit.pb.go
#	api/mesh/v1alpha1/retry.pb.go
#	api/mesh/v1alpha1/service_insight.pb.go
#	api/mesh/v1alpha1/timeout.pb.go
#	api/mesh/v1alpha1/traffic_log.pb.go
#	api/mesh/v1alpha1/traffic_permission.pb.go
#	api/mesh/v1alpha1/traffic_route.pb.go
#	api/mesh/v1alpha1/traffic_trace.pb.go
#	api/mesh/v1alpha1/zone_ingress.pb.go
#	api/mesh/v1alpha1/zone_ingress.proto
#	api/mesh/v1alpha1/zoneingress_overview.pb.go
#	pkg/xds/auth/callbacks_test.go
@mergify mergify bot mentioned this pull request Jul 2, 2021
Merged
jakubdyszkiewicz added a commit that referenced this pull request Jul 2, 2021
@jakubdyszkiewicz
chore(kuma-cp) upgrade Protocol Buffers (#2244)
96b33fc 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
jakubdyszkiewicz pushed a commit that referenced this pull request Jul 5, 2021
@mergify
chore(kuma-cp) upgrade Protocol Buffers (#2244)
1703da3 
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpeach jpeach jpeach approved these changes

@bartsmykla bartsmykla bartsmykla approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jakubdyszkiewicz @codecov-commenter @jpeach @bartsmykla