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

Reject version skew between gRPC client and server #1141

Merged
merged 3 commits into from
Aug 14, 2020
Merged

Reject version skew between gRPC client and server #1141

merged 3 commits into from
Aug 14, 2020

Conversation

mogren
Copy link
Contributor

@mogren mogren commented Aug 14, 2020 •
edited
Loading

Issue #, if available:
Rebase of #1137 and #1139

Description of changes:
Rebased on master and regenerated protobufs using the same protoc as previous update.

  • Regenerate protobufs using same version as master
  • Reject version skew between gRPC client and server (test in this comment)
  • Regenerate mocks and update method name

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mogren mogren requested review from anguslees and jayanthvn August 14, 2020 17:31
@mogren mogren force-pushed the anguslees-versionlock branch from e712280 to ff32b7e Compare August 14, 2020 18:07
mogren
mogren commented Aug 14, 2020
View reviewed changes
Copy link
Contributor Author

@mogren mogren left a comment

Choose a reason for hiding this comment

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

@anguslees LGTM! :)

pkg/ipamd/rpc_handler.go
Comment on lines +229 to +230
func (c *IPAMContext) RunRPCHandler(version string) error {
log.Infof("Serving RPC Handler version %s on %s", version, ipamdgRPCaddress)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

jayanthvn
jayanthvn approved these changes Aug 14, 2020
View reviewed changes
Copy link
Contributor

@jayanthvn jayanthvn left a comment

Choose a reason for hiding this comment

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

Looks good :)

anguslees and others added 3 commits August 14, 2020 11:39
@anguslees
Regenerate mocks and update method name
389501a 
Follow-on to be9ca94

be9ca94 renamed `GetENIipLimit` to
`GetENIIPv4Limit`, but didn't regenerate the mock tests.  More
surprisingly, we had a test that was still asserting the wrong method
was called successfully :(

Run `make generate` and update the resulting build failure in
ipamd_test.
@anguslees
Reject version skew between gRPC client and server
7f7dea3 
Include the client version in gRPC requests, and reject requests on
the server (ipamd) side if they don't match.  This prevents potential
version skew between client and server.

(ie: Previously we could have a brief race during aws-node daemonset
upgrade/downgrade - between restarting ipamd and replacing the old CNI
binary.  This will now result in an error, and the kubelet will retry
the sandbox add/del.)
Regenerate protobufs using same version as master
8a3abfa
@mogren mogren force-pushed the anguslees-versionlock branch from ff32b7e to 8a3abfa Compare August 14, 2020 18:39
@jayanthvn jayanthvn added this to the v1.7.0 milestone Aug 14, 2020
@jayanthvn jayanthvn merged commit 8e61bec into aws:master Aug 14, 2020
This was referenced Aug 14, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jayanthvn jayanthvn jayanthvn approved these changes

@anguslees anguslees Awaiting requested review from anguslees

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@mogren @jayanthvn @anguslees