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 submodule to latest master in microsoft/main #1285

Merged

Conversation

microsoft-golang-bot
Copy link
Collaborator

Hi! I'm a bot, and this is an automatically generated upstream sync PR. 🔃

After submitting the PR, I will attempt to enable auto-merge in the "merge commit" configuration.

For more information, visit sync documentation in microsoft/go-infra.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! Auto-approving.

@dagood
Copy link
Member

dagood commented Jul 31, 2024

Some test failures with the opensslcrypto and cngcrypto backends.

Note that oddly, linux-amd64 test (ubuntu) [opensslcrypto] didn't fail, even though it's configured to use opensslcrypto.

golang/go@650abbc

@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 2 times, most recently from 1745d08 to 17b4f51 Compare August 5, 2024 16:08
@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 4 times, most recently from 6cc7182 to b62a604 Compare August 14, 2024 16:07
@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 3 times, most recently from 0ba1681 to 17a456c Compare August 21, 2024 16:07
@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 2 times, most recently from 721b803 to 1c7ea02 Compare August 26, 2024 16:07
@karianna
Copy link
Member

Some test failures with the opensslcrypto and cngcrypto backends.

Note that oddly, linux-amd64 test (ubuntu) [opensslcrypto] didn't fail, even though it's configured to use opensslcrypto.

golang/go@650abbc

Should we be concerned about this drift or do we reconcile closer to a release?

@dagood
Copy link
Member

dagood commented Aug 26, 2024

On July 23, the upstream master branch opened for 1.24 development, and branch reopening generally seems to cause a small spike in commits that have higher amounts of incompatibilities than usual. This PR opened after that, July 31, so this seems expected. This is also the furthest away we'll be from the next major version, so it works out. 😄

Going after conflicts/errors as soon as possible is always better overall. The only potential waste that can happen often is if there's an upstream issue, someone finds it and fixes it, but we spent time investigating without realizing that.

Quim normally deals with resolving test issues, so I might not have a full picture historically.

I do have a submodule update PR open that gets a bit further and runs into a known issue(/missing feature) that will need to be done: #1289. I think it can wait until Quim is available. (Even if I implement it, to review.) /cc @gdams

@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 3 times, most recently from 3500877 to 77d001d Compare September 4, 2024 16:08
@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 4 times, most recently from 6f080b0 to d6d341c Compare September 13, 2024 16:06
@qmuntal qmuntal mentioned this pull request Sep 18, 2024
@microsoft-golang-bot microsoft-golang-bot force-pushed the dev/auto-sync/microsoft/main branch 3 times, most recently from 6886a2c to 495cae7 Compare September 23, 2024 16:07
@qmuntal
Copy link
Contributor

qmuntal commented Sep 27, 2024

Note that upstream added TLS1.3 support when TLS FIPS-only mode is enabled: https://go-review.googlesource.com/c/go/+/603376. This means that we can drop the 0010-Support-TLS-1.3-in-fipstls-mode patch.

On the other hand, upstream doesn't support the P-521 curve because it is not a recommended curve to be used with TLS 1.3, although it is allowed. We have historically allowed it to be aligned with Windows and OpenSSL supported curves. Now we have another data point to support this stance: the SymCrypt provider for OpenSSL also allows P-521 (see here).

Back to this PR: I'll remove the aforementioned patch so we don't conflict with the incoming changes. I'll then submit a follow up PR that reenables P-521.

@dagood
Copy link
Member

dagood commented Sep 27, 2024

Restored 07e4fd0. (git fetch msft 07e4fd05bb4c3deb46641f055a573d7f8a729d49; git push msft 07e4fd05bb4c3deb46641f055a573d7f8a729d49:refs/heads/dev/auto-sync/microsoft/main -f.)

@qmuntal
Copy link
Contributor

qmuntal commented Sep 27, 2024

I'll take a look at #68 today.

That would be very much appreciated!

@microsoft-golang-review-bot microsoft-golang-review-bot merged commit 2005ada into microsoft/main Sep 30, 2024
22 checks passed
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.

5 participants