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

ci: update to Go 1.15.4 and alpine:3.12 #9036

Merged
merged 5 commits into from
Nov 13, 2020
Merged

Conversation

mikemorris
Copy link
Contributor

Is there any substantial reason we can't/shouldn't bump to Go 1.15.x for the Consul 1.9 release series? Did I miss bumping the version in any files?

@mikemorris mikemorris added this to the 1.9 milestone Oct 26, 2020
@mikemorris mikemorris requested a review from a team October 26, 2020 18:00
@github-actions github-actions bot added the type/ci Relating to continuous integration (CI) tooling for testing or releases label Oct 26, 2020
@mikemorris
Copy link
Contributor Author

In the go-test-api TestAPI_ClientTLSOptions, several tests are failing with x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0

@mikemorris mikemorris requested a review from a team October 26, 2020 19:55
@mikemorris
Copy link
Contributor Author

It looks like maybe static validation of time is more strict in 1.15, not sure if this simply means two of these failing tests could be removed now...

TestSession_Apply_BadTTL
    session_endpoint_test.go:929: incorrect error message: Session TTL '10z' invalid: time: unknown unit "z" in duration "10z"
TestConfigUtil_Values
    config_test.go:85: (case 3) err: 1 error(s) decoding:
        
        * error decoding 'duration': time: invalid duration "nope"

@rboyer
Copy link
Member

rboyer commented Oct 28, 2020

@mikemorris I fixed the two tests in question. The error messages are mostly the same, except that now the inputs are wrapped in double quotes now so our assertions were slightly incorrect.

@rboyer
Copy link
Member

rboyer commented Oct 28, 2020

Now there's this fun one with multiple occurrences:

 CONT  TestSetupHTTPServer_HTTP2
    http_test.go:196: err: Get "https://127.0.0.1:38331/echo": x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0

@rboyer
Copy link
Member

rboyer commented Oct 28, 2020

might be good to wait until 1.15.4 for golang/go#42138

@mikemorris mikemorris modified the milestones: 1.9.0, 1.9.0-beta2 Oct 28, 2020
@rboyer
Copy link
Member

rboyer commented Oct 28, 2020

I also regenerated some of the TLS test data files so they don't anger the updated tls stack in Go.

@mikemorris
Copy link
Contributor Author

Yeaaaa, x509: certificate relies on legacy Common Name field is my biggest concern with moving to 1.15, it feels like a change we might want to look into a little more closely, particularly implications on service mesh things...

@mikemorris
Copy link
Contributor Author

mikemorris commented Oct 28, 2020

might be good to wait until 1.15.4 for golang/go#42138

Sooo, clock skew is potentially not fun, and we just published a consul:1.8.5 Docker image bundling a binary with alpine:3.12, and the actual pipeline building the binary would've been using circleci/golang:1.14.9 which is FROM golang:1.14.9 and appears to use either alpine:3.11 or alpine 3.12

I'm slightly unclear on if the issue is only present when running a compiled binary on an affected version of alpine or if the issue is which version of alpine was used to compile the binary, but it looks like 1.8.5 could be affected...

/cc @alvin-huang

@rboyer
Copy link
Member

rboyer commented Oct 28, 2020

Yeaaaa, x509: certificate relies on legacy Common Name field is my biggest concern with moving to 1.15, it feels like a change we might want to look into a little more closely, particularly implications on service mesh things...

Service mesh doesn't use the Common Name, preferring instead the newer SubjectAlternativeName (SAN) field. The former is singly valued and the latter is multi valued.

@rboyer
Copy link
Member

rboyer commented Oct 29, 2020

This lets you easily display the common name and subject alternative name fields from the CLI:

$ openssl x509  -noout -in server.crt -subject -ext subjectAltName

@mikemorris mikemorris changed the title chore: update to Go 1.15.3 chore: update to Go 1.15.4 Nov 6, 2020
@mikemorris mikemorris changed the title chore: update to Go 1.15.4 ci: update to Go 1.15.4 and alpine:3.12 Nov 11, 2020
@mikemorris mikemorris merged commit 7af643a into master Nov 13, 2020
@mikemorris mikemorris deleted the chore/golang-1.15.3 branch November 13, 2020 18:03
@hashicorp-ci
Copy link
Contributor

🍒 Starting backport cherry picking.

To cherry-pick post-merge, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/280920.

@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 7af643a onto release/1.9.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Nov 13, 2020
* ci: stop building darwin/386 binaries

Go 1.15 drops support for 32-bit binaries on Darwin https://golang.org/doc/go1.15#darwin

* tls: ConnectionState::NegotiatedProtocolIsMutual is deprecated in Go 1.15, this value is always true

* correct error messages that changed slightly

* Completely regenerate some TLS test data

Co-authored-by: R.B. Boyer <rb@hashicorp.com>
rboyer added a commit that referenced this pull request Apr 14, 2021
rboyer added a commit that referenced this pull request Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/ci Relating to continuous integration (CI) tooling for testing or releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants