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

Use AWS SDK v2 for Go #1938

Closed
matt0x6F opened this issue Apr 8, 2024 · 7 comments · Fixed by #1950
Closed

Use AWS SDK v2 for Go #1938

matt0x6F opened this issue Apr 8, 2024 · 7 comments · Fixed by #1950
Assignees
Labels
good first issue Great issues for new Athenians to work on! refactor work to do to refactor code
Milestone

Comments

@matt0x6F
Copy link
Contributor

matt0x6F commented Apr 8, 2024

Is your feature request related to a problem? Please describe.

The existing AWS SDK we use is in maintenance mode. We should migrate to v2 when we can. No official dates for migration have been issued at the time of writing.

Describe the solution you'd like

Reimplement the AWS SDK with the new version.

Describe alternatives you've considered

  • Not upgrading

Additional context

N/A

@matt0x6F matt0x6F added good first issue Great issues for new Athenians to work on! refactor work to do to refactor code labels Apr 8, 2024
@nesangcode
Copy link
Contributor

Can I assigned to this issue?

@matt0x6F
Copy link
Contributor Author

Can I assigned to this issue?

Yeap! Go for it.

@matt0x6F matt0x6F assigned matt0x6F and nesangcode and unassigned matt0x6F Apr 13, 2024
@nesangcode
Copy link
Contributor

In s3_test.go, it'll skip getStorage test if ATHENS_MINIO_ENDPOINT not set up. How can I test getStorage() without set ATHENS_MINIO_ENDPOINT?

func getStorage(t testing.TB) *Storage {
	url := os.Getenv("ATHENS_MINIO_ENDPOINT")
	if url == "" {
		t.SkipNow()
	}
	...
}

@matt0x6F
Copy link
Contributor Author

@k124k3n try the docker based e2e tests. There's a target in the Makefile that correlates to this.

@nesangcode
Copy link
Contributor

Seem's like AWS SDK v2 require protocol:// in Endpoint.

When I set export ATHENS_MINIO_ENDPOINT="127.0.0.1:9001", it fails on s3_test.go:

s3_test.go:104: operation error S3: CreateBucket, resolve auth scheme: resolve endpoint: endpoint rule error, Custom endpoint `127.0.0.1:9001` was not a valid URI

When I set export ATHENS_MINIO_ENDPOINT="http://127.0.0.1:9001", it fails on minio_test.go:

--- FAIL: TestBackend (0.00s)
    minio_test.go:107: address http://127.0.0.1:9001: too many colons in address
--- FAIL: TestNewStorageExists (0.00s)
    minio_test.go:39: TestNewStorageExists failed for bucketname:  testbucket, error: address http://127.0.0.1:9001: too many colons in address

Should we add protocol:// in aws.Config.Endpoint?

@matt0x6F
Copy link
Contributor Author

@k124k3n since Minio is the only one that expects there not to be a protocol in the address could we make part of the function trim prefixes for http:// and https://?

@matt0x6F matt0x6F added this to the 0.14.0 milestone Apr 20, 2024
@nesangcode
Copy link
Contributor

Sure, also, we should update Athens S3 documentation to refer into AWS SDK v2 docs.

matt0x6F pushed a commit that referenced this issue Apr 29, 2024
Upgrades the AWS SDK to v2. AWS S3 bucket urls will now error if they are not prefixed with a schema (example: https://).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Great issues for new Athenians to work on! refactor work to do to refactor code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants