Release/26.1.5/uploadfile by alhaustin · Pull Request #174 · skyflowapi/skyflow-go

alhaustin · 2026-03-25T22:20:43Z

Base64 File Upload Fails in Read-Only Container Filesystems

* SK-2506: add support for custom token uri

…ules

…ng-rules-and-fix-hardcoded-values-in-go-v2-sdk SK-2520: extract hard coded strings to constants and update linting r…

* SK-2502 fix the request and response in connections * SK-2502 close response in connections * SK-2541 fix the test * SK-2541 resolve the comment

* SK-2645: allow nil and empty values in insert and update request

Copilot

Pull request overview

This PR introduces/expands SDK functionality across v1 and v2 modules (client/services, constants, Insert BYOT + ContinueOnError behavior, and a new Get API), while also updating Go module versions and CI/security workflows and adding multiple samples/docs. The current diff set does not clearly target the stated issue (“Base64 File Upload Fails in Read-Only Container Filesystems”) and appears broader in scope.

Changes:

Add v2 module scaffolding (client services, constants, tests) and new samples
Extend v1 vault APIs (Insert BYOT/ContinueOnError, Detokenize bulk mode, new Get API) plus tests and message constants
Update Go versions and CI/security workflows (linters, CodeQL/Semgrep/GoSec/Gitleaks, Codecov)

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
v2/internal/constants/constants.go	Adds a centralized constants set used by v2 code.
v2/go.mod	Introduces v2 module definition and dependencies.
v2/client/vault_service.go	Adds v2 Vault service wrapper methods delegating to controllers.
v2/client/detect_service.go	Adds v2 Detect service wrapper delegating to controller.
v2/client/connection_service.go	Adds v2 Connection service wrapper delegating to controller.
v2/client/client_test.go	Adds/expands v2 client initialization and config management tests.
v2/client/client.go	Implements v2 Skyflow client + option/config management and service builders.
v2/.golangci.yml	Adds golangci-lint configuration for v2 module.
skyflow/vaultapi/insert_test.go	Adds tests for Insert ContinueOnError and BYOT validation scenarios.
skyflow/vaultapi/insert.go	Adds BYOT validation, ContinueOnError request/response handling, and request_index.
skyflow/vaultapi/get_test.go	Adds tests for the new v1 Get API validation and request flows.
skyflow/vaultapi/get.go	Introduces a new v1 Get API with concurrent requests per record.
skyflow/vaultapi/detokenize_test.go	Adds a test for detokenize options/bulk behavior.
skyflow/vaultapi/detokenize.go	Adds DetokenizeOptions and implements bulk detokenize when ContinueOnError=false.
skyflow/go.mod	Updates v1 skyflow module Go version and dependencies (incl. renamed serviceaccount module).
skyflow/common/helpers.go	Adds ConvertToMaps helper used in new bulk detokenize path.
skyflow/common/common.go	Adds BYOT enum, extends options structs, and adds response structs for Insert/Get.
skyflow/client/client_test.go	Updates v1 client tests for BYOT and detokenize options; fixes import path.
skyflow/client/client.go	Adds Insert default BYOT, Detokenize options wiring, and new Get client method.
serviceaccount/util/token_test.go	Updates env var usage for private key in token tests.
serviceaccount/util/token.go	Switches JWT dependency to jwt/v4.
serviceaccount/go.mod	Updates Go version and switches JWT dep to jwt/v4; dependency bumps.
samples/v2/vaultapi/upload_file.go	Adds v2 sample showing UploadFile usage.
samples/v2/vaultapi/update_record.go	Adds v2 Update record sample.
samples/v2/vaultapi/tokenize_records.go	Adds v2 Tokenize sample.
samples/v2/vaultapi/query_record.go	Adds v2 Query sample.
samples/v2/vaultapi/invoke_connection.go	Adds v2 Invoke Connection sample.
samples/v2/vaultapi/insert_records.go	Adds v2 Insert sample.
samples/v2/vaultapi/insert_byot.go	Adds v2 BYOT insert sample.
samples/v2/vaultapi/get_records.go	Adds v2 Get-by-IDs sample.
samples/v2/vaultapi/get_column_values.go	Adds v2 Get-by-column-values sample.
samples/v2/vaultapi/detokenize.go	Adds v2 Detokenize sample.
samples/v2/vaultapi/delete.go	Adds v2 Delete sample.
samples/v2/serviceaccount/token_generation_with_context.go	Adds v2 serviceaccount sample with context.
samples/v2/serviceaccount/token_generation_example.go	Adds v2 token generation sample.
samples/v2/serviceaccount/signed_token_generation.go	Adds v2 signed token generation sample.
samples/v2/serviceaccount/scoped_token_generation.go	Adds v2 scoped token generation sample.
samples/v2/serviceaccount/bearer_token_expiry_example.go	Adds v2 sample for retrying on 401 token expiry.
samples/v2/go.mod	Adds a dedicated go.mod for v2 samples module.
samples/v1/vaultapi/insert_with_continueOnError.go	Adds v1 sample for Insert ContinueOnError usage.
samples/v1/vaultapi/get_with_options.go	Adds v1 sample for Get with options.
samples/v1/vaultapi/get.go	Adds v1 sample for Get (IDs + column-values).
samples/v1/vaultapi/detokenize.go	Updates v1 detokenize sample for ContinueOnError option.
samples/v1/go.mod	Adds a dedicated go.mod for v1 samples module.
samples/go.mod	Removes old samples module go.mod.
go.mod	Updates root Go version.
commonutils/messages/messages.go	Adds message strings for Get API, detokenize bulk, BYOT, and partial success.
commonutils/go.mod	Updates commonutils Go version and dependencies.
README-v1.md	Adds/updates v1 documentation including Get/BYOT/ContinueOnError details.
CHANGELOG.md	Adds changelog entries for versions 1.8.0–1.10.0.
.github/workflows/semgrep.yml	Adds Semgrep scanning workflow and PR commenting.
.github/workflows/pull_request_template.md	Adds PR template content.
.github/workflows/main.yml	Updates CI to newer Go, adds vendoring, runs v1+v2 tests, updates Codecov config.
.github/workflows/codeql-analysis.yml	Updates CodeQL workflow tooling and build steps.
.github/workflows/ci.yml	Updates CI Go version, adds v2 linting, runs v1+v2 tests, updates Codecov config.
.github/workflows/GoSec.yml	Adds GoSec scanning workflow and PR commenting.
.github/workflows/Gitleaks.yml	Adds Gitleaks scanning workflow and PR commenting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T23:08:06Z

v2/internal/constants/constants.go

 	SDK_LOG_PREFIX         = "[ " + SDK_PREFIX + " ] "
 	SDK_NAME               = "Skyflow Go SDK "
 	SDK_VERSION            = "v2.0.4"
 	SDK_PREFIX             = SDK_NAME + SDK_VERSION


SDK_LOG_PREFIX references SDK_PREFIX before SDK_PREFIX is declared in the same const block. In Go, constants can only reference identifiers declared earlier in the same const block, so this will not compile. Reorder the constants so SDK_PREFIX is declared before SDK_LOG_PREFIX (or inline the value), then run go test to confirm compilation.

Suggested change

SDK_NAME = "Skyflow Go SDK "

SDK_VERSION = "v2.0.4"

SDK_PREFIX = SDK_NAME + SDK_VERSION

SDK_LOG_PREFIX = "[ " + SDK_PREFIX + " ] "

Copilot · 2026-03-25T23:08:06Z

v2/client/client.go

This stores the address of the range loop variable vaultConfig (&vaultConfig) into maps. In Go, the range variable is reused across iterations, so all entries will end up pointing to the same final value. Create a per-iteration copy (e.g., vc := vaultConfig) and store &vc (and similarly for detect) to avoid incorrect configs at runtime.

Copilot · 2026-03-25T23:08:07Z

v2/client/client.go

Same range-variable-address bug as in WithVaults: storing &connectionConfig will cause all entries to reference the last iteration’s config. Copy per iteration and store a pointer to the copy to ensure each connection service retains the correct config.

Suggested change

// create a per-iteration copy to avoid taking the address of the range variable

cfg := connectionConfig

// create the connection service

s.connectionServices[connectionConfig.ConnectionId] = &connectionService{

config: &cfg,

Copilot · 2026-03-25T23:08:07Z

v2/client/client.go

RemoveVault deletes from vaultServices before verifying the corresponding entry exists in detectServices. If detectServices[vaultId] is missing, the method returns an error but has already mutated state (vault removed, detect not removed), leaving the client inconsistent and potentially causing panics later (e.g., in UpdateVault). Validate both maps first (or delete both without a second existence check after mutation), so the operation is atomic from the caller’s perspective.

Suggested change

if _, exists := s.detectServices[vaultId]; !exists {

logger.Error(fmt.Sprintf(logs.VAULT_ID_CONFIG_DOES_NOT_EXIST, vaultId))

return error.NewSkyflowError(error.ErrorCodesEnum(error.INVALID_INPUT_CODE), error.VAULT_ID_NOT_IN_CONFIG_LIST)

}

delete(s.vaultServices, vaultId)

Copilot · 2026-03-25T23:08:07Z

skyflow/vaultapi/get.go

If url.Parse(requestUrl) returns an error, this goroutine never sends anything on responseChannel, but the parent loop still waits for len(records.Records) responses, causing a deadlock. Ensure the err != nil path sends an error object into responseChannel (and returns) so the consumer loop can always make progress.

Copilot · 2026-03-25T23:08:08Z

samples/v2/vaultapi/upload_file.go

WithCredentials in v2 validates credentials; passing common.Credentials{} will likely fail client initialization and makes this sample non-runnable even after the user fills in vault tokens in VaultConfig. Either remove WithCredentials(...) from samples where each vault config already has credentials, or replace it with a placeholder that is actually valid (and clearly instruct users to set it).

Suggested change

Copilot · 2026-03-25T23:08:08Z

skyflow/vaultapi/get_test.go

This JSON is invalid (missing a comma after "card_pin": "123"). The test won’t exercise the intended happy path and may instead hit error-handling behavior. Fix the JSON so it’s valid and assert the returned error/response to ensure the test verifies correct behavior.

Copilot · 2026-03-25T23:08:09Z

skyflow/vaultapi/detokenize_test.go

The response JSON is malformed (there’s an unmatched ] and object braces don’t balance). This can cause JSON unmarshal errors that mask what the test is trying to validate. Replace this with valid JSON matching the API error schema and assert on the returned error/records accordingly.

Copilot · 2026-03-25T23:08:09Z

v2/client/client_test.go

The assertion contradicts the test comment: it says an invalid credential should return an error, but it expects nil. Either the comment is wrong or the expectation is wrong; as written this test can allow credential-validation regressions to slip through. Align the assertion with the intended behavior (likely Expect(errr1).ShouldNot(BeNil())).

Suggested change

Expect(errr1).ShouldNot(BeNil())

Copilot · 2026-03-25T23:08:09Z

v2/client/vault_service.go

The PR description is about “Base64 File Upload Fails in Read-Only Container Filesystems”, but the reviewed changes don’t show any update to the actual base64 upload implementation (e.g., avoiding filesystem writes, using temp dirs, or streaming). This change is just a pass-through wrapper; if the fix is elsewhere it isn’t included in this diff set. Consider narrowing the PR scope or adding the missing UploadFile/base64 handling changes and targeted tests that reproduce a read-only filesystem failure.

github-actions · 2026-03-25T23:09:07Z

🔐 Gitleaks Findings: 1 issue(s) detected

🔸 Rule: generic-api-key
📄 File: v2/internal/vault/controller/controller_test.go:null
📝 Description: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
🔑 Secret: **********
🔗 Path: v2/internal/vault/controller/controller_test.go:null

github-actions · 2026-03-25T23:09:23Z

GoSec Findings: No issues found, Good to merge.

github-actions · 2026-03-25T23:09:36Z

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

saileshwar-skyflow and others added 7 commits January 27, 2026 21:41

SK-2506: Add support for custom tokenUri (#163)

b0e1d5d

* SK-2506: add support for custom token uri

SK-2520: extract hard coded strings to constants and update linting r…

1c0d500

…ules

SK-2520: addressed copilot review comments

9885b7f

Merge pull request #164 from skyflowapi/himanshu/SK-2520-update-linti…

17ed090

…ng-rules-and-fix-hardcoded-values-in-go-v2-sdk SK-2520: extract hard coded strings to constants and update linting r…

SK-2502 fix the request and response in connections (#165)

56a4f75

* SK-2502 fix the request and response in connections * SK-2502 close response in connections * SK-2541 fix the test * SK-2541 resolve the comment

SK-2645: Merge branch 'release/26.1.1' into release/26.1.5

7724065

SK-2645: Allow nil and empty values in insert and update request (#172)

20af05e

* SK-2645: allow nil and empty values in insert and update request

alhaustin changed the base branch from main to release/23.9.1 March 25, 2026 22:23

alhaustin marked this pull request as draft March 25, 2026 22:24

jstjoe requested a review from Copilot March 25, 2026 23:03

jstjoe changed the base branch from release/23.9.1 to main March 25, 2026 23:07

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Merge branch 'main' into release/26.1.5

fcb87ce

+			// create a per-iteration copy to avoid taking the address of the range variable
+			cfg := connectionConfig
+			// create the connection service
+			s.connectionServices[connectionConfig.ConnectionId] = &connectionService{
+				config:   &cfg,

+	if _, exists := s.detectServices[vaultId]; !exists {
+		logger.Error(fmt.Sprintf(logs.VAULT_ID_CONFIG_DOES_NOT_EXIST, vaultId))
+		return error.NewSkyflowError(error.ErrorCodesEnum(error.INVALID_INPUT_CODE), error.VAULT_ID_NOT_IN_CONFIG_LIST)
+	}
+	delete(s.vaultServices, vaultId)

Conversation

alhaustin commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants