feat: Go V3 test server; rename Java and Python as "V3" implementations by lucasmcdonald3 · Pull Request #4 · aws/amazon-s3-encryption-client-python

lucasmcdonald3 · 2025-09-16T17:32:24Z

Issue #, if available:

Description of changes:

Add Go test server
Rename Python and Java servers to "V3" -- we'll be doing lots of cross-version testing so let's set up the versioning now

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copilot

Pull Request Overview

This PR adds a Go V3 test server implementation and renames existing Java and Python servers to explicitly indicate they are "V3" implementations, setting up the framework for cross-version testing of S3 Encryption Client implementations.

Add complete Go V3 test server with HTTP endpoints for client creation and S3 object operations
Rename Java and Python servers from generic names to "V3" specific versions
Update test infrastructure to support the new Go server and handle encryption context validation differences

Reviewed Changes

Copilot reviewed 7 out of 29 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test-server/java-v3-server/README.md	Updates documentation to reflect V3 naming
test-server/java-tests/src/it/java/software/amazon/encryption/s3/RoundTripTests.java	Adds Go-V3 server support and new encryption context validation tests
test-server/go-v3-server/main.go	New Go test server implementation with full S3EC functionality
test-server/go-v3-server/go.mod	Go module definition with required dependencies
test-server/go-v3-server/README.md	Documentation for the new Go test server
test-server/README.md	Updated instructions for V3 server naming
test-server/Makefile	Updated build targets to support all three V3 servers

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-16T18:13:17Z

+	ctx := context.Background()
+	encryptionContext := context.WithValue(ctx, "EncryptionContext", encCtx)


Similar to the PUT operation, the encryption context is being added to the Go context but may not be used by the S3 encryption client for validation during decryption. The comment on lines 309-310 suggests this implementation doesn't validate encryption context, which may be due to this incorrect usage.

Suggested change

ctx := context.Background()

encryptionContext := context.WithValue(ctx, "EncryptionContext", encCtx)

Correct, I could go either way here.
I'll probably leave it in so the test server behavior explicitly mirrors that of other test servers, but will defer to reviewers' opinions.

kessplas · 2025-09-16T18:16:53Z

+
+	// Generate client ID
+	clientID := uuid.New().String()
+	log.Printf("CreateClient: Generated client ID: %s", clientID)


I'm fine with keeping some print logging, but I think we should be more concise than we are now.

Also, let's add a [Go v3] prefix or similar to differentiate it from other servers.

Reduced logging to only either log error or success

…s3/RoundTripTests.java Co-authored-by: Kess Plasmeier <76071473+kessplas@users.noreply.github.com>

…client-python into go-test-server

kessplas

LGTM, please have Shubham take a look as well

ShubhamChaturvedi7

Nothing major; Approving since this PR is building block of other changes that need to go, the comments can be addressed as a follow up PR.

ShubhamChaturvedi7 · 2025-09-16T23:51:08Z

+
+- **HTTP Framework**: Gorilla Mux for routing
+- **AWS SDK**: AWS SDK for Go v2 for S3 and KMS operations
+- **Concurrency**: Thread-safe client caching with sync.RWMutex


Is it being used anywhere? I see the cache is plain map (which I guess works; see my comment below).

Oops sorry AI slop slipped through, will remove

ShubhamChaturvedi7 · 2025-09-16T23:53:51Z

+)
+
+// Server represents the Go test server
+type Server struct {


nit / optional: you can make them all private (structs) if it's all in the same package.

ShubhamChaturvedi7 · 2025-09-17T00:04:00Z

+	keyring := materials.NewKmsKeyring(kmsClient, input.Config.KeyMaterial.KMSKeyID, func(options *materials.KeyringOptions) {
+		options.EnableLegacyWrappingAlgorithms = input.Config.EnableLegacyWrappingAlgorithms


What about other values in the input config? Do we not use them?

The existing Java and Python implementations don't, so I'm mirroring those

ShubhamChaturvedi7 · 2025-09-17T00:10:04Z

+        serverList = new ArrayList<>(14);
+        serverList.add(new LanguageServerTarget("Java-V3", "8080"));
+        serverList.add(new LanguageServerTarget("Python-V3", "8081"));
+        serverList.add(new LanguageServerTarget("Go-V3", "8082"));
+
+        serverMap = new HashMap<>(14);
+        serverMap.put("Java-V3", new LanguageServerTarget("Java-V3", "8080"));
+        serverMap.put("Python-V3", new LanguageServerTarget("Python-V3", "8081"));
+        serverMap.put("Go-V3", new LanguageServerTarget("Go-V3", "8082"));


Why list and map both? It'd be a bit less maintenance if it was just a map (and iterate over the valueSet).

I'm not sure but I'll leave this as-is

ShubhamChaturvedi7 · 2025-09-17T00:28:58Z

+	clientID := uuid.New().String()
+
+	// Store client in cache
+	s.clientCache[clientID] = s3EncryptionClient


So we are safe against multi-threading because each request would just create a new uuid. Cool.

lucasmcdonald3 · 2025-09-17T16:41:13Z

I'll merge since my only change since 2 approvals is cleaning up the README.

2e594b6

Lucas McDonald added 2 commits September 16, 2025 10:31

m

8522b26

m

4d44e4f

lucasmcdonald3 changed the title ~~feat: Go V3 test server~~ feat: Go V3 test server; rename Java and Python as "V3" implementations Sep 16, 2025

m

6622859

lucasmcdonald3 requested a review from Copilot September 16, 2025 18:12

Copilot AI reviewed Sep 16, 2025

View reviewed changes

kessplas reviewed Sep 16, 2025

View reviewed changes

lucasmcdonald3 and others added 4 commits September 16, 2025 11:17

Update test-server/java-tests/src/it/java/software/amazon/encryption/…

bcae379

…s3/RoundTripTests.java Co-authored-by: Kess Plasmeier <76071473+kessplas@users.noreply.github.com>

Update test-server/java-tests/src/it/java/software/amazon/encryption/…

bf0683c

…s3/RoundTripTests.java Co-authored-by: Kess Plasmeier <76071473+kessplas@users.noreply.github.com>

m

afecd75

Merge branch 'go-test-server' of github.com:aws/amazon-s3-encryption-…

d122c1c

…client-python into go-test-server

kessplas approved these changes Sep 16, 2025

View reviewed changes

ShubhamChaturvedi7 approved these changes Sep 17, 2025

View reviewed changes

m

2e594b6

lucasmcdonald3 merged commit 2316fa4 into fireegg-test-servers Sep 17, 2025
2 checks passed

lucasmcdonald3 deleted the go-test-server branch September 17, 2025 16:46

		ctx := context.Background()
		encryptionContext := context.WithValue(ctx, "EncryptionContext", encCtx)

		keyring := materials.NewKmsKeyring(kmsClient, input.Config.KeyMaterial.KMSKeyID, func(options *materials.KeyringOptions) {
		options.EnableLegacyWrappingAlgorithms = input.Config.EnableLegacyWrappingAlgorithms

Conversation

lucasmcdonald3 commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kessplas left a comment

Choose a reason for hiding this comment

Uh oh!

ShubhamChaturvedi7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasmcdonald3 commented Sep 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lucasmcdonald3 commented Sep 16, 2025 •

edited

Loading