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

service/s3 typo in api_client.go results in undefined: presignedurlcust.AddAsIsPresigingMiddleware #2529

Closed
tonygooldrr opened this issue Mar 4, 2024 · 7 comments · Fixed by #2530
Labels
bug This issue is a bug. p0 This issue is the highest priority

Comments

@tonygooldrr
Copy link

Describe the bug

I cannot compile using the current SDK because of what appears to be a typo. "Presiging" should be "Presigning".

Expected Behavior

The project compiles.

Current Behavior

The project fails to compile with the following error:

/go/pkg/mod/github.com/aws/aws-sdk-go-v2/service/s3@v1.51.1/api_client.go:855:25: undefined: presignedurlcust.AddAsIsPresigingMiddleware

Reproduction Steps

Attempt a go build on the following main.go:

package main

import (
	"context"
	"log"

	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
)

func main() {
	cfg, _ := config.LoadDefaultConfig(context.Background())
	client := s3.NewFromConfig(cfg)
	if client != nil {
		log.Println("Initialized client")
	}
}

Possible Solution

Fix the typo so it reads presignedurlcust.AddAsIsPresigningMiddleware.

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2/config v1.27.5
github.com/aws/aws-sdk-go-v2/service/s3 v1.51.1

Compiler and Version used

go version go1.22.0 darwin/arm64

Operating System and version

macOS 14.3

@tonygooldrr tonygooldrr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 4, 2024
@lucix-aws
Copy link
Contributor

lucix-aws commented Mar 4, 2024

@tonygooldrr Really sorry about this -- it's actually the other way around -- the typo was fixed today, and now your s3 service is out-of-date. If you upgrade your s3 client the issue should go away.

go get -u github.com/aws/aws-sdk-go-v2/...

@lucix-aws lucix-aws added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 4, 2024
@lucix-aws
Copy link
Contributor

lucix-aws commented Mar 4, 2024

This is my fault. We received an external contribution to correct the typo in the internal package from AddAsIsPresigingMiddleware to AddAsIsPresigningMiddleware. I accepted the patch, believing that despite the technical API break it would be a non-issue since the module was under an internal namespace and therefore impossible to import directly by a caller.

I failed to consider the scenario where a user could be broken if they depended on two other SDK modules that use this module, and upgraded only one of them. You have appeared to hit that with the service/s3 and config modules - config@v1.27.5 is from today, so it depends on the new version of presigned-url with the typo fix, but service/s3@v1.51.1 is from yesterday, so it's still expecting the old typo'd version.

We will guard ourselves against these changes going forward. I've documented this as a cross-module sharp edge internally.

@lucix-aws lucix-aws added compatibility Issues with SDK, Go language version, or platform compatibility. and removed bug This issue is a bug. labels Mar 4, 2024
@lucix-aws
Copy link
Contributor

FYI - I'm going to restore this as an alias to the now correctly-named method to preserve backwards compatibility going forward. See the linked PR. That will go out in tomorrow's release.

I'll go ahead and mark this as resolved once that PR has merged.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 5, 2024
@lucix-aws lucix-aws added bug This issue is a bug. p0 This issue is the highest priority and removed compatibility Issues with SDK, Go language version, or platform compatibility. labels Mar 5, 2024
Copy link

github-actions bot commented Mar 5, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@der-eismann
Copy link

Hey @lucix-aws, thanks for the insights and the quick fix. Just to give you some insights as well, basically all of our ~30 dependabot PRs failed today because of this. You mentioned:

I failed to consider the scenario where a user could be broken if they depended on two other SDK modules that use this module, and upgraded only one of them

However our dependabot job is updating all direct dependencies, so this can't be the issue. I guess it's hidden in the transitive dependencies somewhere, in one case we only have these two as direct dependencies:

	github.com/aws/aws-sdk-go-v2/config v1.27.5
	github.com/aws/aws-sdk-go-v2/service/ecr v1.27.1

This pulled in github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.3 // indirect and github.com/aws/aws-sdk-go-v2/service/sts v1.28.2 // indirect, however the indirect S3 dependency was not updated and thus failed with

# github.com/aws/aws-sdk-go-v2/service/s3
vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_client.go:828:25: undefined: presignedurlcust.AddAsIsPresigingMiddleware

So we're looking forward to the next release and dependabot run, hoping it works then without manual intervention 🙂

@tonygooldrr
Copy link
Author

@lucix-aws I can confirm this is fixed in s3 v1.51.2, thanks for the quick turnaround!

@lucix-aws
Copy link
Contributor

However our dependabot job is updating all direct dependencies, so this can't be the issue.

My example was just the most straightforward case. Any dependency shuffling that upgraded only some of the modules consuming the module with the breakfix would hit this error, yours is an example of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p0 This issue is the highest priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants