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.

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

update all deps #1501

Merged
merged 3 commits into from
Jul 31, 2019
Merged

update all deps #1501

merged 3 commits into from
Jul 31, 2019

Conversation

srenatus
Copy link
Contributor

#1411 with the protobuf code updated, too.

@srenatus srenatus self-assigned this Jul 30, 2019
@srenatus
Copy link
Contributor Author

I'm afraid this is one of my first experiences with go modules 😅 Some thorough review of what I've done here would be appreciated.

@bonifaido
Copy link
Member

Do we have some tests for the Proto client API? :)

@srenatus
Copy link
Contributor Author

Do we have some tests for the Proto client API? :)

There's an example client that does a few calls:

make bin/dex
make bin/grpc-client
env SAN=IP.1:127.0.0.1 examples/grpc-client/cert-gen
bin/dex serve examples/grpc-client/config.yaml &
bin/grpc-client -ca-crt ca.crt  -client-crt client.crt -client-key client.key

but I don't think that this is wired up with travis or anything.

Also, thanks for getting me to try this, we've got a small bug in error reporting for VerifyPassword:

time="2019-07-30T12:11:37Z" level=info msg="api: password check failed : %vcrypto/bcrypt: hashedPassword is not the hash of the given password"

☝️This is what the server logs. Looks like we're using Info() somewhere Infof() should be used.

@srenatus
Copy link
Contributor Author

srenatus commented Jul 30, 2019

#1502 ✔️

Copy link
Member

@bonifaido bonifaido left a comment

Choose a reason for hiding this comment

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

But if it works now after the manual test than all good, maybe we could add some protobuf based exercise to the tests later on.

@srenatus
Copy link
Contributor Author

I'm still hesitant to merge this. This PR is +354,709 −83,649, #1411 was +294,875 −75,858. Am I doing something wrong? 🤔

@bonifaido
Copy link
Member

Please try go mod tidy and then go build and you should have the current correct state in go.mod and go.sum (and of course go mod vendor again).

@srenatus
Copy link
Contributor Author

I'll look into this tomorrow.

Signed-off-by: Stephan Renatus <[email protected]>
@srenatus
Copy link
Contributor Author

So, I've run only go get -u; make revendor once again, and adapted to the prometheus API change, and regenerated the PB code. I've also reviewed the docs on modules, I think we're OK doing what we do. I'd rather get rid of the vendor.go "trick" eventually, but it's probably OK for now.

I'm ready to assume that the five months that have passed since #1411 are the reason for the difference... 😄

@bonifaido
Copy link
Member

I'm ready to assume that the five months that have passed since #1411 are the reason for the difference... 😄

This is totally normal 😄 👍

@srenatus srenatus merged commit 6ae11a1 into master Jul 31, 2019
@srenatus srenatus deleted the sr/bump-all-deps branch July 31, 2019 07:01
mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants