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 to Go 1.24.0 #5880

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update to Go 1.24.0 #5880

wants to merge 5 commits into from

Conversation

rturner3
Copy link
Collaborator

@rturner3 rturner3 commented Feb 19, 2025

  • Update RSA keys used in tests to have >= 1024 bits. rsa.GenerateKey() in Go 1.24 now throws errors when a key size < 1024 bits is used: https://pkg.go.dev/crypto/rsa#hdr-Minimum_key_size
  • Update internal fork of golang.org/x/crypto/acme/autocert/internal/acmetest to the version from v0.33.0. There are changes to this file that are required for Go 1.24 compatibility, see CL for reference. Existing changes to the fork of this file were preserved.
    • One additional forked change was required to disable a newly added behavior upstream that causes any tests that encounter HTTP errors in the CA to fail. This was required to preserve our existing unit test TestACMEAuth/new-account-tos-not-accepted from pkg/server/endpoints/bundle/server_test.go that verifies some of the forked behavior around ACME CA Terms of Service being accepted. This deviation was annotated with a comment in the forked pkg/server/endpoints/bundle/internal/acmetest/ca.go file.
  • Rework the TestACMEAuth subtests in pkg/server/endpoints/bundle/server_test.go to accommodate newer behavior of the forked acmetest CA from golang.org/x/crypto. acmetest.CAServer now has a Start() method that must be called to generate the root CA certificate and URL for the fake server. This method also registers a closer method as a test cleanup method. This resulted in a few changes to the tests:
    • Every subtest now needs its own acmetest.CAServer since they are not reusable across tests due to the closer method being registered as a test cleanup method
    • The cached subtest needed to be merged with the initial subtest because it depended on acmetest.CAServer state that can no longer be shared across subtests
  • Suppress new staticcheck linter warnings related to importing deprecated OPA v0 packages

@sorindumitru
Copy link
Collaborator

Looks like there some extra changes in the diff from merges, would you mind rebasing again to clean things up a bit?

@rturner3
Copy link
Collaborator Author

Looks like there some extra changes in the diff from merges, would you mind rebasing again to clean things up a bit?

Hmm, one of the merges must have gotten messed up. I just rebased on main, I think it's in a better state now.

Signed-off-by: Ryan Turner <[email protected]>
sorindumitru
sorindumitru previously approved these changes Feb 24, 2025
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.

3 participants