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

Upgrade Go version to latest #1897

Closed

Conversation

markmandel
Copy link
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:

It had been a while, and there were some security fixes, so figured it was worth doing again.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

I've not incremented example image versions, since I figured this wasn't
a breaking change, and the new version could be picked up as other
changes happen to those images.

It had been a while, and there were some security fixes, so figured it
was worth doing again.

I've not incremented example image versions, since I figured this wasn't
a breaking change, and the new version could be picked up as other
changes happen to those images.
@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory labels Nov 12, 2020
@google-cla google-cla bot added the cla: yes label Nov 12, 2020
@markmandel markmandel added the area/security Issues pertaining to security label Nov 12, 2020
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 9c156fce-05d4-44fb-8d29-b5fd4fad1729

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, roberthbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [markmandel,roberthbailey]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 4751ec8d-9b69-4dae-94b8-ca9272da22bb

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 39b85d55-8d80-4e7e-9db1-7456a24a6924

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@markmandel
Copy link
Collaborator Author

Something in

--- FAIL: TestFleetAutoscalerTLSWebhook (368.86s)
    fleetautoscaler_test.go:640: error waiting for fleet condition on fleet simple-fleet-p72kt
FAIL
time="2020-11-12 23:14:11.772" level=info msg="Namespace 1605222429 is deleted"
FAIL	agones.dev/agones/test/e2e	422.848s

Seems to consistently fail. Will have to dig in and see what it might be?

@markmandel
Copy link
Collaborator Author

Running the tests locally, I'm getting this error:

  Warning  FleetAutoscaler  70s (x4 over 74s)  fleetautoscaler-controller  Error calculating desired fleet size on FleetAutoscaler simple-fleet-9wbw8-autoscaler. Error: Post "https://autoscaler-tls-service.default.svc:8000/scale": dial tcp 10.3.242.176:8000: connect: connection refused
  Warning  FleetAutoscaler  9s (x21 over 69s)  fleetautoscaler-controller  Error calculating desired fleet size on FleetAutoscaler simple-fleet-9wbw8-autoscaler. Error: Post "https://autoscaler-tls-service.default.svc:8000/scale": x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0

Seems like a change in cert handling! Will need to work out what is happening there.

@markmandel markmandel added the kind/breaking Breaking change label Nov 13, 2020
@markmandel
Copy link
Collaborator Author

Found context on this one. Apparently it's a 20 (?) year old deprecation that has decided to be enforced with this version of Go:
golang/go#39568
golang/go#39568 (comment) < gives a bit more details.

In theory we should be able to update our test certs, and it'll be fine - but we should also make sure to update our documentation to match when creating certs for webhooks - and a troubleshooting guide would also help just in case.

@markmandel
Copy link
Collaborator Author

I'm going to close this, and upgrade to 1.14.x for the security fixes -- for the life of me, I cannot get this to work by either:

  • incorporating GODEBUG=x509ignoreCN=0
  • Updating certs to using SANs rather than Common Name

Filing an issue to upgrade to 1.15.x and migrate webhooks to SAN based certs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory area/security Issues pertaining to security cla: yes kind/breaking Breaking change kind/cleanup Refactoring code, fixing up documentation, etc lgtm size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants