[Experimental] lambda experiment #296
Conversation
WalkthroughThis update spans multiple modules and vendor packages. It updates the Go version and dependency revisions in Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as CLI Command
participant L as Lambda Main Command
participant R as staticLambdaResolver
participant LC as lambdaConnectorClient
participant LS as Lambda Service
U->>C: Invoke Lambda command
C->>L: Execute MakeLambdaMainCommand
L->>R: Resolve Lambda endpoint
R-->>L: Return resolved URL
L->>LC: Establish Lambda connector client
LC->>LS: Send invocation request
LS-->>LC: Return response
LC-->>L: Pass result back to CLI
L-->>C: Command completes with output
sequenceDiagram
participant App as Application
participant CW as clientOnlyWrapper
participant CC as NewConnectorClient
participant Reg as Register (service registration)
App->>CW: Initialize client-only wrapper
CW->>CC: Create ConnectorClient via NewConnectorClient
CC-->>CW: Return ConnectorClient
CW->>Reg: Invoke Register with RegisterOps options
Reg-->>CW: Registration complete (or error)
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (1)
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.go (1)
59-76: Update mergeOverrides to handle ImplicitGlobalRegion.The mergeOverrides function needs to be updated to handle the new ImplicitGlobalRegion field to maintain consistency with other fields.
type RegionOverrides struct { Name *string `json:"name"` DnsSuffix *string `json:"dnsSuffix"` DualStackDnsSuffix *string `json:"dualStackDnsSuffix"` SupportsFIPS *bool `json:"supportsFIPS"` SupportsDualStack *bool `json:"supportsDualStack"` + ImplicitGlobalRegion *string `json:"implicitGlobalRegion"` } func mergeOverrides(into PartitionConfig, from RegionOverrides) PartitionConfig { // ... existing code ... + if from.ImplicitGlobalRegion != nil { + into.ImplicitGlobalRegion = *from.ImplicitGlobalRegion + } return into }
🧹 Nitpick comments (25)
vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.go (2)
15-17: Consider embedding atomic.Int64 instead of storing a pointer.Storing a pointer to
atomic.Int64can lead to null checks throughout the code. Embedding the atomic type directly in the struct may simplify usage and reduce the risk of nil-pointer errors.type AddTimeOffsetMiddleware struct { - Offset *atomic.Int64 + Offset atomic.Int64 }
22-31: Inconsistent receiver usage between HandleBuild and ID.
HandleBuilduses a value receiver (func (m AddTimeOffsetMiddleware) ...), whileIDandHandleDeserializeuse pointer receivers. It's more consistent to use pointer receivers across the board, especially if you're mutating or accessing shared state likeOffset.-func (m AddTimeOffsetMiddleware) HandleBuild(ctx context.Context, ... +func (m *AddTimeOffsetMiddleware) HandleBuild(ctx context.Context, ...vendor/github.com/aws/aws-lambda-go/lambda/sigterm.go (1)
46-48: Typographical error in the comment.There's a small spelling mistake in "initalizing" which should be "initializing." Correcting this helps keep the codebase polished.
-// ... let the API know we're done initalizing. +// ... let the API know we're done initializing.vendor/github.com/aws/aws-lambda-go/lambda/panic.go (1)
66-85: Correct repeated spelling mistakes for “separators.”The comment uses "seperators" instead of "separators" in multiple places, which can create minor confusion.
-// Strip GOPATH from path by counting the number of seperators in label & path +// Strip GOPATH from path by counting the number of separators in label & path -// Something went wrong and path has less seperators than we expected +// Something went wrong and path has fewer separators than we expectedvendor/github.com/aws/aws-lambda-go/lambda/rpc_function.go (1)
35-46: Graceful termination afterrpc.Accept
Currently, after callingrpc.Accept(lis), the function immediately returns an error, relying onrpc.Acceptnever terminating under normal operation. Consider adding either a more descriptive error message or a graceful shutdown path ifrpc.Acceptexits unexpectedly.vendor/github.com/aws/aws-lambda-go/lambda/entry.go (2)
88-97: Deprecation approach
TheStartHandlerWithContextfunction is marked deprecated but still publicly callable. If you must keep it for backward compatibility, consider providing explicit documentation or instructing users to migrate to the recommended alternative (StartWithOptions).
99-111: No fallback if no supported environment variable is defined
Thestartfunction expects at least one of the known environment variables (AWS_LAMBDA_RUNTIME_API, etc.). If none are present, it callslogFatalf. Depending on usage, you might want to offer a fallback path or a more specific guideline in the error message for developers who accidentally run this code locally.pkg/config/lambda_config.go (1)
88-99: Maintaining deterministic field ordering
When iterating over the merged fields, the code places them into a map and then re-inserts them into a new slice. This approach ensures uniqueness but can lose the original field order. If field ordering is important, consider preserving it with a stable merging technique.vendor/github.com/aws/aws-lambda-go/lambda/runtime_api_client.go (1)
12-12: Consider modern alternatives toioutil.The
ioutilpackage is deprecated in recent Go releases. Although anolintdirective is present, switching toio(io.Discardorosutilities) can future-proof this code and avoid suppressing linters.- "io/ioutil" //nolint: staticcheck + "io"vendor/github.com/aws/aws-lambda-go/lambda/invoke_loop.go (2)
25-28: Clarify the TODO milestone.
The comment references dropping support for Go versions prior to 1.17 before switching totime.UnixMilli. Consider adding a clear milestone or GitHub issue link so the intended future change is well documented.
104-117: Limit or mask error details in logs.
reportFailurelogs error payloads directly. If these payloads can contain sensitive data, consider adding partial masking or redaction to protect PII or confidential info.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_AddPermission.go (2)
50-124: Confirm required fields are handled consistently.
Several struct fields inAddPermissionInputare marked as required in the docs. Make sure these are validated, either client-side or via the AWS service response, to avoid runtime errors.
137-235: Keep operation middlewares consistent.
addOperationAddPermissionMiddlewaresconfigures a long pipeline. If other operations follow a similar pattern, consider extracting common middlewares to shared helpers for clarity and maintainability.pkg/cli/lambda_command.go (1)
38-42: Leverage concurrency or caching for endpoint resolution.
staticLambdaResolveris straightforward but repeatedly callingResolveEndpointmay re-parse the same URL. Consider caching or reusing if repeated calls are expected.vendor/github.com/aws/aws-lambda-go/lambda/handler.go (3)
12-12: Consider usingiopackage instead ofioutil.
io/ioutilis deprecated in newer Go versions. Consider switching to"io"and usingio.ReadAllfor better forward compatibility and to avoid the lint suppression comment.Example refactor:
- "io/ioutil" // nolint:staticcheck + "io"
120-120: Correct the doc comment for WithDisallowUnknownFields.The comment text says:
“WithUseNumber sets the DisallowUnknownFields option on the underlying json decoder.”
But the function name is
WithDisallowUnknownFields(...).Adjust the comment to match the function name and behavior.-// WithUseNumber sets the DisallowUnknownFields option on the underlying json decoder +// WithDisallowUnknownFields sets the DisallowUnknownFields option on the underlying json decoder
270-270: Switch fromioutil.ReadAlltoio.ReadAll.Go Best Practices recommend
io.ReadAllfor readability and forward compatibility. This avoids the deprecation notice onioutil.ReadAll.-b, err := ioutil.ReadAll(response) +b, err := io.ReadAll(response)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_client.go (1)
259-262: Check for correct fallback on metrics creation failure.If
withOperationMetricsreturns an error, the code aborts early. Confirm this does not mask essential behavior. If metrics creation is optional, consider logging a warning instead of returning an error.vendor/github.com/aws/aws-lambda-go/lambda/entry_generic.go (1)
13-21: LGTM! Clean generic implementation for type-safe Lambda handlers.The implementation provides compile-time type safety for Lambda handlers while maintaining backward compatibility.
Consider documenting examples of how to use these generic handlers in your Lambda functions to help other developers understand the benefits of type safety.
vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.go (1)
43-46: Enhance function documentation.Consider adding more detailed documentation explaining:
- The purpose and impact of clock skew in AWS requests
- The expected format and valid range of the duration value
- Example usage
vendor/github.com/aws/aws-lambda-go/lambdacontext/context.go (1)
30-40: Consider error handling for memory limit parsing.The memory limit parsing could benefit from more robust error handling.
if limit, err := strconv.Atoi(os.Getenv("AWS_LAMBDA_FUNCTION_MEMORY_SIZE")); err != nil { - MemoryLimitInMB = 0 + // Log the error for debugging purposes + MemoryLimitInMB = -1 // Use -1 to indicate parsing error } else { MemoryLimitInMB = limit }vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.go (1)
159-169: Enhance ARN parsing robustness.The ARN parsing could be more robust and better documented.
-// extract accountID from arn with format "arn:partition:service:region:account-id:[resource-section]" +// getAccountID extracts the AWS account ID from an assumed role ARN. +// ARN format: arn:partition:service:region:account-id:resource +// Returns empty string if the ARN is nil or not in the expected format. func getAccountID(u *types.AssumedRoleUser) string { if u.Arn == nil { return "" } parts := strings.Split(*u.Arn, ":") - if len(parts) < 5 { + // Verify ARN has at least 6 parts (arn:partition:service:region:account-id:resource) + if len(parts) != 6 { return "" } + // Validate account ID format (12-digit number) + if !regexp.MustCompile(`^\d{12}$`).MatchString(parts[4]) { + return "" + } return parts[4] }pkg/field/defaults.go (1)
53-53: Consider documenting the purpose of empty relationship slices.While initializing empty relationship slices is valid, it would be helpful to add comments explaining their intended use and whether they will be populated later.
Also applies to: 62-62
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.go (1)
200-227: Consider enhancing error handling for address parsing.The error from
net.SplitHostPortis silently ignored. While the current implementation gracefully falls back to setting the full address, it might be helpful to log parsing failures for debugging purposes.Consider this enhancement:
if err != nil { // don't blow up just because we couldn't parse + if span.IsRecording() { + span.AddEvent("address_parse_error", tracing.WithAttributes( + tracing.String("error", err.Error()), + )) + } span.SetProperty("net.peer.addr", raddr.String()) } else {vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.go (1)
315-323: Consider optimizing memory allocation in buildFeatureMetrics.The current implementation makes multiple allocations for the string slice and join operation. Consider using a strings.Builder for better performance with large feature sets.
Here's an optimized version:
func buildFeatureMetrics(features map[UserAgentFeature]struct{}) string { fs := make([]string, 0, len(features)) for f := range features { fs = append(fs, string(f)) } sort.Strings(fs) - return fmt.Sprintf("%s/%s", FeatureMetadata2.string(), strings.Join(fs, ",")) + var b strings.Builder + b.Grow(len(fs)*2 + 4) // Approximate size: features + commas + "m/" prefix + b.WriteString(FeatureMetadata2.string()) + b.WriteByte('/') + for i, f := range fs { + if i > 0 { + b.WriteByte(',') + } + b.WriteString(f) + } + return b.String() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
go.sumis excluded by!**/*.sumvendor/github.com/ductone/c1-lambda/pb/c1/connectorapi/baton/v1/config.pb.gois excluded by!**/*.pb.govendor/github.com/ductone/c1-lambda/pb/c1/connectorapi/baton/v1/config_grpc.pb.gois excluded by!**/*.pb.govendor/github.com/ductone/c1-lambda/pb/c1/transport/v1/transport.pb.gois excluded by!**/*.pb.govendor/github.com/pquerna/xjwt/go.workis excluded by!**/*.workvendor/github.com/pquerna/xjwt/go.work.sumis excluded by!**/*.sumvendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (82)
go.mod(5 hunks)internal/connector/connector.go(5 hunks)pkg/cli/commands.go(1 hunks)pkg/cli/lambda_command.go(1 hunks)pkg/config/config.go(1 hunks)pkg/config/lambda_config.go(1 hunks)pkg/connectorrunner/runner.go(1 hunks)pkg/field/defaults.go(1 hunks)vendor/github.com/aws/aws-lambda-go/LICENSE(1 hunks)vendor/github.com/aws/aws-lambda-go/LICENSE-LAMBDACODE(1 hunks)vendor/github.com/aws/aws-lambda-go/LICENSE-SUMMARY(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/README.md(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/entry.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/entry_generic.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/errors.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/extensions_api_client.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/handler.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/handlertrace/trace.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/invoke_loop.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/messages/README.md(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/messages/messages.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/panic.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/rpc_function.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/runtime_api_client.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambda/sigterm.go(1 hunks)vendor/github.com/aws/aws-lambda-go/lambdacontext/README.md(1 hunks)vendor/github.com/aws/aws-lambda-go/lambdacontext/context.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/config.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.go(4 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics/metrics.go(0 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.go(9 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.go(8 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.go(3 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.go(0 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.go(4 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.go(3 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/config.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/env_config.go(6 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/load_options.go(6 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/provider.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/resolve.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.go(5 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.go(3 hunks)vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/path_value.go(0 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.go(2 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.go(7 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.json(3 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/CHANGELOG.md(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/LICENSE.txt(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_client.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_AddLayerVersionPermission.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_AddPermission.go(1 hunks)vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateAlias.go(1 hunks)
⛔ Files not processed due to max files limit (45)
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateEventSourceMapping.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateFunction.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateFunctionUrlConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteAlias.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteEventSourceMapping.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteFunction.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteFunctionCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteFunctionConcurrency.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteFunctionEventInvokeConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteFunctionUrlConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteLayerVersion.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_DeleteProvisionedConcurrencyConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetAccountSettings.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetAlias.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetEventSourceMapping.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunction.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionConcurrency.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionConfiguration.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionEventInvokeConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionRecursionConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetFunctionUrlConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetLayerVersion.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetLayerVersionByArn.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetLayerVersionPolicy.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetPolicy.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetProvisionedConcurrencyConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_GetRuntimeManagementConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_Invoke.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_InvokeAsync.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_InvokeWithResponseStream.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListAliases.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListCodeSigningConfigs.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListEventSourceMappings.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListFunctionEventInvokeConfigs.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListFunctionUrlConfigs.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListFunctions.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListFunctionsByCodeSigningConfig.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListLayerVersions.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListLayers.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListProvisionedConcurrencyConfigs.go
- vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_ListTags.go
💤 Files with no reviewable changes (3)
- vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.go
- vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics/metrics.go
- vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/path_value.go
✅ Files skipped from review due to trivial changes (20)
- vendor/github.com/aws/aws-lambda-go/lambda/README.md
- vendor/github.com/aws/aws-lambda-go/lambda/messages/README.md
- vendor/github.com/aws/aws-lambda-go/lambdacontext/README.md
- vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.go
- vendor/github.com/aws/aws-lambda-go/LICENSE-SUMMARY
- vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.go
- vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.go
- vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.go
- vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.go
- pkg/config/config.go
- vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.go
- vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.md
- vendor/github.com/aws/aws-lambda-go/LICENSE-LAMBDACODE
- vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.md
- vendor/github.com/aws/aws-lambda-go/LICENSE
- vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.md
- vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.md
- vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.md
- vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.go
- vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.md
🧰 Additional context used
🪛 golangci-lint (1.62.2)
pkg/config/lambda_config.go
109-109: appendAssign: append result not assigned to the same slice
(gocritic)
pkg/cli/lambda_command.go
340-340: use of fmt.Println forbidden by pattern ^(fmt\.Print(|f|ln)|print|println)$
(forbidigo)
47-47: non-wrapping format verb for fmt.Errorf. Use %w to format errors
(errorlint)
go.mod
117-117: local replacement are not allowed: github.com/ductone/c1-lambda
(gomoddirectives)
🪛 LanguageTool
vendor/github.com/aws/aws-sdk-go-v2/service/lambda/CHANGELOG.md
[style] ~322-~322: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...entation of various input fields across a large number of services. Calling code that references ...
(LARGE_NUMBER_OF)
[uncategorized] ~562-~562: The official spelling of this programming framework is “Node.js”.
Context: ...-05-12) * Feature: Lambda releases NodeJs 16 managed runtime to be available in a...
(NODE_JS)
vendor/github.com/aws/aws-sdk-go-v2/service/lambda/LICENSE.txt
[uncategorized] ~151-~151: Possible missing comma found.
Context: ...iateness of using or redistributing the Work and assume any risks associated w...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~162-~162: ‘any and all’ might be wordy. Consider a shorter alternative.
Context: ...ge, computer failure or malfunction, or any and all other commercial damages or losse...
(EN_WORDINESS_PREMIUM_ANY_AND_ALL)
🔇 Additional comments (74)
vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.go (1)
33-42: Concurrent offset updates are efficiently handled.The use of
atomic.Int64ensures thread-safe writes and reads to the offset, preventing data races when the middleware is reused or invoked concurrently.vendor/github.com/aws/aws-lambda-go/lambda/sigterm.go (1)
17-25: Single invocation of SIGTERM handler.Currently, the goroutine reads from the
signaledchannel exactly once, which handles only the first SIGTERM. Subsequent SIGTERMs won't trigger additional handlers. This may be intentional for Lambda, but consider whether repeated signals should be handled.vendor/github.com/aws/aws-lambda-go/lambda/rpc_function.go (1)
98-99: Potential concurrency risk with process-wide environment variable
Usingos.Setenv("_X_AMZN_TRACE_ID", req.XAmznTraceId)is process-wide and not goroutine-scoped. In highly concurrent scenarios, multiple invokes could overwrite each other’s trace ID. If concurrency in your Lambda environment is possible, consider storing this trace ID in a context or a thread-safe structure to prevent possible data races.pkg/config/lambda_config.go (1)
109-109: Static analysis false positive for append usage
Theappendcall on line 109 is assigned to a new slice variable (defaultFields), which is valid. The lint rule (appendAssign) may misinterpret this usage. You can safely ignore it unless you suspect inadvertent mutations of the underlying slice.🧰 Tools
🪛 golangci-lint (1.62.2)
109-109: appendAssign: append result not assigned to the same slice
(gocritic)
vendor/github.com/aws/aws-lambda-go/lambda/runtime_api_client.go (2)
43-43: Double-check indefinite timeouts.Setting
Timeout: 0on thehttp.Clientmeans no upper limit on these requests. While this may be intentional for Lambda RPC, in some cases it may introduce reliability concerns if the runtime API becomes unresponsive. Ensure this is the intended behavior.
161-173: Check error handling inerrorCapturingReader.When a read error occurs (other than EOF), the code sets a trailer and then returns
io.EOF. This can mask the original error from upstream consumers that may rely on explicit error checking. Confirm that this omission of error details is acceptable and that callers won't need to distinguish partial reads from non-retryable errors.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_AddLayerVersionPermission.go (2)
37-78: Ensure required fields are verified before usage.
Action,LayerName,Principal,StatementId, andVersionNumberare documented as required but aren't explicitly validated before making the API call. Relying solely on server-side validation is acceptable but consider local checks to catch user errors earlier if feasible.
94-127: Overall implementation looks solid.The middleware chain and error handling logic appear consistent with other operations in this package. No further issues spotted.
vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_CreateAlias.go (2)
37-74: Validate required fields locally if needed.
FunctionName,FunctionVersion, andNameare mandatory for creating an alias. Although the AWS backend likely validates these, adding optional local validation can provide an earlier failure and clearer error messaging if these fields are omitted or malformed.
107-205: No immediate issues identified.The middleware stack setup follows a familiar pattern. The code provides a straightforward means to create an alias with optional routing configuration. Looks good overall.
vendor/github.com/aws/aws-lambda-go/lambda/invoke_loop.go (3)
30-43: Confirm restart logic is not required.
startRuntimeAPILooploops indefinitely to process invocations. If a non-recoverable error occurs, it returns. Confirm whether you need a retry mechanism that restarts the API loop or gracefully handles transitions if the function environment is reused.
45-84: Ensure correct environment cleanup.
handleInvokesets the_X_AMZN_TRACE_IDenv variable for each invocation and usescontext.WithValuefor trace ID. On repeated invocations, confirm that leftover trace data won't interfere with subsequent requests.
119-130: Validate panic-handling strategy.
callBytesHandlerFuncrecovers from panics but returns an error indicating a forced shutdown. Double-check whether continuing to run after a panic is acceptable or if you want the process to shut down.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_op_AddPermission.go (1)
35-48: Handle nil input gracefully.
AddPermissionreplaces a nil input with&AddPermissionInput{}, which is fine for optional fields. Ensure downstream validations occur—especially for required fields.pkg/cli/lambda_command.go (1)
52-74: Check concurrency-safety oflambdaConnectorClient.
Multiple goroutines may call this function. Ensure AWS SDK config loading and usage of the sharedlambdaClientis concurrency-safe, or create separate clients as needed.vendor/github.com/aws/aws-lambda-go/lambda/handler.go (1)
23-34: Assess concurrency safety formap[interface{}]interface{}.The
handlerOptionsstruct holdscontextValuesas a standard map. If multiple goroutines modify it post-initialization, data races could occur. If mutation is done only once at initialization, it should be safe. Otherwise, consider using a sync.Map or locking.vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.go (3)
93-96: Validate concurrency on metric creation.
newAttemptMetrics(r.OperationMeter)allocates new metrics for each retry loop. This is generally safe, but ifOperationMeteris shared across goroutines, confirm that metric creation is thread-safe within the meter provider.
230-231: Ensure consistent error-wrapping for clock skew.
err = wrapAsClockSkew(ctx, err)conditionally wraps the error based on clock skew codes. Confirm that all possible skew-related error codes are handled or default to existing logic. This approach is beneficial; just verify it doesn’t mask broader errors.
411-418: Validate error codes inerrorType.The function attempts to return
terr.ErrorCode()if available, or the Go type otherwise. This is sound. Just ensure complex wrapped errors still surface the correct code. The fallback to Go type helps with diagnosing non-API errors.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/api_client.go (2)
167-169: Thread safety fortimeOffsetfield.The
timeOffsetusesatomic.Int64; ensure all reads and writes (including any increments or sets) go through atomic operations or remain read-only to avoid data races.
217-224: Returning a copy of Options is good practice.The
Options()method returns a fresh copy, preventing unintended side effects on the client’s runtime config. This is an excellent approach for immutability and safe reusability.vendor/github.com/aws/aws-lambda-go/lambda/entry_generic.go (1)
1-2: Verify Go version compatibility in the codebase.The file requires Go 1.18+ for generics support. Ensure your codebase's Go version is compatible.
✅ Verification successful
Go Version Compatibility Verified.
- The
go.modfile specifies Go version 1.23.4, which exceeds the minimum requirement (Go 1.18) for generics.- The CI workflow configurations use
"go-version": 1.22.x, which also satisfies the generics requirement.No compatibility issues were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Go version in go.mod and CI configuration echo "Checking Go version in go.mod:" grep -E "^go [0-9]+\.[0-9]+" go.mod echo -e "\nChecking Go version in CI config:" fd -t f "\.ya?ml$" .github/workflows/ -x grep -l "go-version" {} \; -exec grep -A 2 "go-version" {} \;Length of output: 539
Script:
#!/bin/bash # New approach: Recursively search CI configuration files for "go-version" echo "Checking CI configuration for go-version settings:" grep -R "go-version" .github/workflows/ || echo "No go-version settings found in CI workflows."Length of output: 524
vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.go (1)
4-18: LGTM! Well-structured endpoint mode configuration.The implementation provides clear control over AWS account ID handling in endpoint routing with well-documented options.
Let's verify if any existing code relies on these endpoint modes:
✅ Verification successful
Endpoint Mode Usage Verified Across Codebase
I’ve verified that the newAccountIDEndpointModetype and its constants are referenced throughout the repository (e.g., in configuration files likevendor/github.com/aws/aws-sdk-go-v2/config/env_config.goandshared_config.go, as well as in various client implementations). The constants are being effectively used for endpoint routing configuration per AWS SDK patterns, and no additional changes are needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of AccountIDEndpointMode in the codebase echo "Searching for AccountIDEndpointMode usage:" rg -l "AccountIDEndpointMode" --type go echo -e "\nSearching for endpoint configuration patterns:" rg "EndpointMode.*=.*" --type goLength of output: 4199
vendor/github.com/aws/aws-lambda-go/lambda/errors.go (1)
35-46: LGTM! Robust panic handling with stack trace.The panic handling implementation properly captures error details and stack traces.
vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.go (1)
4-33: LGTM! Well-structured checksum configuration options.The implementation provides clear control over request and response checksum behavior with well-documented options.
Let's verify the usage of these checksum configurations:
✅ Verification successful
Checksum configuration usage verified.
The constants are actively used in both
vendor/github.com/aws/aws-sdk-go-v2/aws/config.goandvendor/github.com/aws/aws-sdk-go-v2/aws/checksum.go, confirming their intended integration and proper setup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of checksum configurations in the codebase echo "Searching for RequestChecksumCalculation usage:" rg -l "RequestChecksumCalculation" --type go echo -e "\nSearching for ResponseChecksumValidation usage:" rg -l "ResponseChecksumValidation" --type goLength of output: 509
vendor/github.com/aws/aws-lambda-go/lambda/messages/messages.go (2)
36-46: LGTM! Comprehensive error handling implementation.The error implementation includes message, type, stack trace, and exit flag, with proper JSON serialization. The
Error()method provides detailed error information.
13-14: Style suppressions are properly documented.The
//nolint:stylecheckcomments are appropriately used to maintain AWS's naming conventions while suppressing style warnings.Also applies to: 19-20, 36-37, 48-49
vendor/github.com/aws/aws-lambda-go/lambda/handlertrace/trace.go (2)
16-25: LGTM! Safe callback composition implementation.The
callbackComposefunction properly handles nil callbacks and ensures ordered execution.
32-38: LGTM! Proper context value management.The implementation follows Go's context best practices:
- Uses a private type for the context key
- Composes existing callbacks with new ones
- Maintains context immutability
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.go (1)
17-44: LGTM! Well-structured metrics implementation.The implementation includes:
- Proper error handling for each metric creation
- Clear descriptions and appropriate unit labels
- Comprehensive coverage of attempt counts, errors, and durations
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.go (1)
43-46: LGTM! Proper time skew handling implementation.The implementation correctly:
- Retrieves current time using SDK's time function
- Applies context-based time skew adjustment
- Maintains signing time accuracy
vendor/github.com/aws/aws-lambda-go/lambda/extensions_api_client.go (1)
31-33: Reconsider infinite timeout setting.While the comment suggests connections should never timeout, this could lead to resource leaks. Consider setting a high but finite timeout.
- Timeout: 0, // connections to the extensions API are never expected to time out + Timeout: 24 * time.Hour, // Set a reasonable maximum timeoutvendor/github.com/aws/aws-lambda-go/lambdacontext/context.go (2)
15-28: LGTM! Well-documented global variables.The global variables are well-documented and follow AWS Lambda's standard environment variable naming conventions.
71-89: LGTM! Robust context implementation.The context implementation follows Go best practices:
- Uses unexported type for context key to prevent collisions
- Provides clear documentation for usage
- Implements standard context patterns
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.go (1)
132-132: LGTM! Consistent AccountID propagation.The AccountID field is correctly propagated from the provider options to the credentials.
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.go (1)
131-131: LGTM! Consistent internal structure update.The AccountID field addition to GetCredentialsOutput maintains consistency with other credential providers.
vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.go (1)
139-142: LGTM! AccountID extraction from AssumedRoleUser.The AccountID extraction is handled safely with proper nil checks.
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.go (1)
155-155: LGTM: AccountID field properly integratedThe addition of the AccountID field to the credentials is consistent with the broader SDK changes and is correctly populated from the response.
vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.go (1)
94-95: LGTM: Well-documented AccountID field additionThe AccountID field is properly documented and consistently implemented across the SDK.
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.go (1)
228-228: LGTM: Improved timestamp consistency with UTCConverting the time to UTC before formatting ensures consistent behavior across different timezones and regions, which is crucial for SSO token validation.
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.go (2)
122-125: LGTM: Improved documentation for connection errorsThe added comments effectively clarify the relationship between "connection reset" and "use of closed network connection" errors, providing valuable context for maintainers.
210-228: LGTM: Well-implemented clock skew error handlingThe new retryableClockSkewError type is well-structured and follows Go error handling best practices:
- Clear documentation of purpose
- Proper implementation of error interfaces
- Appropriate error wrapping
pkg/field/defaults.go (2)
39-40: LGTM! Lambda client fields are well-defined.The fields are correctly configured with appropriate descriptions and persistence settings. The required flag on
lambdaClientFunctionFieldensures the Lambda function name is provided.
42-43: LGTM! Lambda server fields are properly secured.Both client ID and secret fields are correctly marked as required and persistent, ensuring secure authentication configuration.
vendor/github.com/aws/aws-sdk-go-v2/config/config.go (1)
84-85: LGTM! AccountIDEndpointMode resolver is correctly integrated.The resolver is appropriately added to the defaultAWSConfigResolvers slice and positioned after related configuration resolvers.
vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.go (2)
171-172: LGTM! AccountID field is properly defined.The field is correctly added with appropriate JSON tag mapping to "AccountId" to match AWS conventions.
214-214: LGTM! AccountID is correctly propagated.The AccountID is properly assigned to the credentials struct during retrieval.
vendor/github.com/aws/aws-sdk-go-v2/config/resolve.go (1)
169-183: LGTM! Well-structured resolver implementation.The implementation follows the established pattern for AWS SDK resolvers and properly handles the default case.
vendor/github.com/aws/aws-sdk-go-v2/aws/config.go (1)
166-194: LGTM! Well-documented configuration options.The new configuration fields are thoroughly documented with clear explanations of their behavior and default values.
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.go (1)
74-101: LGTM! Well-organized feature constants.The feature constants are well-documented with clear naming and organized grouping. The use of single-character values for the feature flags is memory-efficient.
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.go (2)
16-21: LGTM: AWS partition configuration updated with implicit global region.The addition of
ImplicitGlobalRegionwith value "us-east-1" for the AWS partition is correct as it's the primary global region.
115-121: LGTM: New AWS region added.The addition of the "ca-west-1" region follows the same pattern as other regions in the AWS partition.
vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.go (1)
311-324: LGTM: Enhanced credentials with account ID information.The changes safely extract the account ID from the assumed role user and include it in the returned credentials.
pkg/cli/commands.go (1)
174-178: Verify the impact of moving connector initialization.Moving the connector initialization after configuration validation changes the error handling flow. While this ensures configuration is valid before creating the connector, verify that this doesn't break any assumptions in the connector initialization that might depend on early creation.
✅ Verification successful
Connector Initialization Flow Verified
The shell script output confirms that within
pkg/cli/commands.gothe configuration validation (field.Validate) consistently occurs before the connector initialization (getconnector). There is no evidence of any parts of the codebase relying on an early connector initialization, so the error handling flow should remain intact.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any dependencies on early connector initialization # Look for patterns where connector is used before configuration validation # Search for connector usage patterns rg -A 5 "getconnector\(" # Search for configuration validation patterns rg -A 5 "field\.Validate\("Length of output: 3196
internal/connector/connector.go (3)
114-134: LGTM: Added client-only wrapper for better separation of concerns.The
clientOnlyWrapperprovides a clean way to handle client-only scenarios without the overhead of server management.
158-200: LGTM: Enhanced service registration with RegisterOps.The
RegisterOpsstruct andRegisterfunction improve modularity by centralizing service registration logic and configuration.
308-326: LGTM: Added helper for connector client creation.The
NewConnectorClientfunction provides a clean way to create a connector client from a gRPC connection.vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.go (1)
398-402: LGTM! Well-documented header case conversion.The implementation correctly handles S3 limitations by converting specific headers to lowercase, with clear references to the related GitHub issues.
vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.go (1)
358-364: LGTM! Enhanced credential resolution logic.The implementation correctly handles both credential sources for ECS containers with clear error messaging when neither source is available.
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.go (1)
1-39: LGTM! Comprehensive documentation improvements.The documentation updates provide clear guidance on:
- SigV4 implementation and usage
- Pre-escaping requirements for request URIs
- Potential issues when using outside the SDK
- References to relevant documentation and examples
pkg/connectorrunner/runner.go (1)
534-534: Consider addressing the TODO comment before merging.The TODO comment indicates a need for refactoring due to code duplication. Since this is an experimental PR marked as "do not merge", this would be a good opportunity to refactor before finalizing the changes.
vendor/github.com/aws/aws-sdk-go-v2/config/provider.go (1)
228-243: LGTM! Clean implementation of account ID endpoint mode provider.The new interface and function follow the established patterns in the codebase and are well-integrated with the existing provider system.
vendor/github.com/aws/aws-sdk-go-v2/config/env_config.go (2)
84-85: LGTM! Well-defined environment variable constants.The new environment variables follow AWS SDK naming conventions and are clearly documented.
509-529: LGTM! Robust validation of account ID endpoint mode values.The function properly validates the input values and provides clear error messages for invalid configurations.
vendor/github.com/aws/aws-sdk-go-v2/config/load_options.go (1)
349-358: LGTM! Well-implemented configuration helper function.The WithAccountIDEndpointMode helper function follows SDK patterns and properly handles empty values.
vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.go (2)
119-120: LGTM! Clear and consistent constant definitions.The new constants follow the established naming patterns and are well-documented.
1190-1208: LGTM! Thorough validation of account ID endpoint mode configuration.The function properly validates the input values and provides clear error messages for invalid configurations.
go.mod (2)
3-5: Update Go Version and Toolchain
The Go version has been updated to1.23.4along with the toolchain set togo1.23.5. This update appears intentional; please ensure that all dependencies and build scripts are compatible with these changes.
9-21: Update Dependency Versions
Several AWS SDK dependencies and related libraries have been updated (e.g. AWS SDK for Go v2, configuration, credentials, and service modules). These updates should bring in new features and bug fixes. Please verify through integration tests that no breaking changes have been introduced.vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.json (3)
12-12: Enhance Region Regex
The"regionRegex"for the AWS partition has been updated to include the"mx"prefix. This change correctly broadens the valid region list.
47-52: Add New AWS Regions
New regions"ap-southeast-5"(Asia Pacific – Malaysia) and"ap-southeast-7"(Asia Pacific – Thailand) have been added. Please ensure that any downstream configurations or integrations are updated to accommodate these additional regions.
207-211: Update AWS-ISO-E Region Configuration
The AWS-ISO-E partition now includes the region"eu-isoe-west-1"with the description"EU ISOE West". This update increases the granularity of region support. Confirm that services interfacing with ISO endpoints handle this new region appropriately.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/CHANGELOG.md (1)
1-300: Comprehensive Changelog Update
The changelog now documents the release history up to versionv1.69.6with detailed notes on dependency updates, bug fixes, and new features. The entries are clear and provide valuable context. Please ensure that these notes remain in sync with the actual changes in the repository and that any future modifications maintain this level of detail.vendor/github.com/aws/aws-sdk-go-v2/service/lambda/LICENSE.txt (1)
1-203: License Text Integrity
This file contains the full text of the Apache License, Version 2.0. As this is a standard legal document, its content—including any punctuation or phrasing that might appear wordy—must remain unchanged to ensure legal accuracy. No modifications are necessary.🧰 Tools
🪛 LanguageTool
[uncategorized] ~151-~151: Possible missing comma found.
Context: ...iateness of using or redistributing the Work and assume any risks associated w...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~162-~162: ‘any and all’ might be wordy. Consider a shorter alternative.
Context: ...ge, computer failure or malfunction, or any and all other commercial damages or losse...(EN_WORDINESS_PREMIUM_ANY_AND_ALL)
| func safeMarshal(v interface{}) []byte { | ||
| payload, err := json.Marshal(v) | ||
| if err != nil { | ||
| v := &messages.InvokeResponse_Error{ | ||
| Type: "Runtime.SerializationError", | ||
| Message: err.Error(), | ||
| } | ||
| payload, err := json.Marshal(v) | ||
| if err != nil { | ||
| panic(err) // never reach | ||
| } | ||
| return payload | ||
| } | ||
| return payload | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent hidden panic in serialization fallback.
safeMarshal panics if the second JSON marshalling fails. Consider returning an error response instead of panicking to ensure you have a robust error path.
| func newStaticLambdaResolver(endpoint string) (lambda_sdk.EndpointResolverV2, error) { | ||
| uri, err := url.Parse(endpoint) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse endpoint: %v", err) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use wrapping verb %w for error.
Static analysis advises using %w with fmt.Errorf("failed to parse endpoint: %v", err). Adopting %w enables error unwrapping in Go.
- return nil, fmt.Errorf("failed to parse endpoint: %v", err)
+ return nil, fmt.Errorf("failed to parse endpoint: %w", err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return nil, fmt.Errorf("failed to parse endpoint: %v", err) | |
| return nil, fmt.Errorf("failed to parse endpoint: %w", err) |
🧰 Tools
🪛 golangci-lint (1.62.2)
47-47: non-wrapping format verb for fmt.Errorf. Use %w to format errors
(errorlint)
| return fmt.Errorf("failed to get connector config: %w", err) | ||
| } | ||
|
|
||
| fmt.Println("Received config: ", spew.Sdump(config)) |
There was a problem hiding this comment.
Avoid using fmt.Println for logging.
According to the static analysis hint, using fmt.Println is restricted. Replace it with structured logging to maintain consistency and better control over log levels.
- fmt.Println("Received config: ", spew.Sdump(config))
+ l.Info("Received config", zap.String("debug", spew.Sdump(config)))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Println("Received config: ", spew.Sdump(config)) | |
| l.Info("Received config", zap.String("debug", spew.Sdump(config))) |
🧰 Tools
🪛 golangci-lint (1.62.2)
340-340: use of fmt.Println forbidden by pattern ^(fmt\.Print(|f|ln)|print|println)$
(forbidigo)
| return stack.Finalize.Insert(&disableHTTPSMiddleware{ | ||
| DisableHTTPS: o.EndpointOptions.DisableHTTPS, | ||
| }, "ResolveEndpointV2", middleware.After) | ||
| } |
There was a problem hiding this comment.
Confirm security impact of DisableHTTPS.
The middleware that flips a request’s scheme to HTTP can expose requests to interception. If this is only for testing, consider doc warnings or environment checks to prevent production usage.
| func getErrorType(err interface{}) string { | ||
| errorType := reflect.TypeOf(err) | ||
| if errorType.Kind() == reflect.Ptr { | ||
| return errorType.Elem().Name() | ||
| } | ||
| return errorType.Name() | ||
| } | ||
|
|
||
| func lambdaErrorResponse(invokeError error) *messages.InvokeResponse_Error { | ||
| if ive, ok := invokeError.(messages.InvokeResponse_Error); ok { | ||
| return &ive | ||
| } | ||
| var errorName string | ||
| if errorType := reflect.TypeOf(invokeError); errorType.Kind() == reflect.Ptr { | ||
| errorName = errorType.Elem().Name() | ||
| } else { | ||
| errorName = errorType.Name() | ||
| } | ||
| return &messages.InvokeResponse_Error{ | ||
| Message: invokeError.Error(), | ||
| Type: errorName, | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to eliminate duplicated error type extraction logic.
The error type extraction logic is duplicated between getErrorType and lambdaErrorResponse.
Consider refactoring to reuse getErrorType:
func lambdaErrorResponse(invokeError error) *messages.InvokeResponse_Error {
if ive, ok := invokeError.(messages.InvokeResponse_Error); ok {
return &ive
}
- var errorName string
- if errorType := reflect.TypeOf(invokeError); errorType.Kind() == reflect.Ptr {
- errorName = errorType.Elem().Name()
- } else {
- errorName = errorType.Name()
- }
return &messages.InvokeResponse_Error{
Message: invokeError.Error(),
- Type: errorName,
+ Type: getErrorType(invokeError),
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getErrorType(err interface{}) string { | |
| errorType := reflect.TypeOf(err) | |
| if errorType.Kind() == reflect.Ptr { | |
| return errorType.Elem().Name() | |
| } | |
| return errorType.Name() | |
| } | |
| func lambdaErrorResponse(invokeError error) *messages.InvokeResponse_Error { | |
| if ive, ok := invokeError.(messages.InvokeResponse_Error); ok { | |
| return &ive | |
| } | |
| var errorName string | |
| if errorType := reflect.TypeOf(invokeError); errorType.Kind() == reflect.Ptr { | |
| errorName = errorType.Elem().Name() | |
| } else { | |
| errorName = errorType.Name() | |
| } | |
| return &messages.InvokeResponse_Error{ | |
| Message: invokeError.Error(), | |
| Type: errorName, | |
| } | |
| } | |
| func getErrorType(err interface{}) string { | |
| errorType := reflect.TypeOf(err) | |
| if errorType.Kind() == reflect.Ptr { | |
| return errorType.Elem().Name() | |
| } | |
| return errorType.Name() | |
| } | |
| func lambdaErrorResponse(invokeError error) *messages.InvokeResponse_Error { | |
| if ive, ok := invokeError.(messages.InvokeResponse_Error); ok { | |
| return &ive | |
| } | |
| return &messages.InvokeResponse_Error{ | |
| Message: invokeError.Error(), | |
| Type: getErrorType(invokeError), | |
| } | |
| } |
| return | ||
| } | ||
| defer res.Body.Close() | ||
| _, _ = io.Copy(ioutil.Discard, res.Body) |
There was a problem hiding this comment.
Fix response body handling.
The response body is being discarded before attempting to decode it with json.NewDecoder.
- _, _ = io.Copy(ioutil.Discard, res.Body)
-
- if res.StatusCode != http.StatusOK {
- err = fmt.Errorf("failed to register extension, got response status: %d %s", res.StatusCode, http.StatusText(res.StatusCode))
- return
- }
-
- err = json.NewDecoder(res.Body).Decode(&response)
+ if res.StatusCode != http.StatusOK {
+ body, _ := io.ReadAll(res.Body)
+ err = fmt.Errorf("failed to register extension, got response status: %d %s: %s", res.StatusCode, http.StatusText(res.StatusCode), string(body))
+ return
+ }
+
+ err = json.NewDecoder(res.Body).Decode(&response)Also applies to: 88-88
| // Deprecated: This structure was used with the global [EndpointResolver] | ||
| // interface, which has been deprecated in favor of service-specific endpoint | ||
| // resolution. See the deprecation docs on that interface for more information. |
There was a problem hiding this comment.
💡 Codebase verification
Deprecated endpoint usage found in pkg/us3/s3.go requires migration.
Our verification revealed that the internal package at pkg/us3/s3.go still creates and uses a custom endpoint resolver via awsSdk.EndpointResolverWithOptionsFunc (lines 260 and 272). This confirms that the codebase is relying on the deprecated endpoint types, which might prevent access to new endpoint-related features. Please plan a migration to service‑specific endpoint resolvers.
- File to inspect:
pkg/us3/s3.go(lines 260, 272)
🔗 Analysis chain
Review deprecation impact on the codebase.
Multiple endpoint-related types are being deprecated in favor of service-specific endpoint resolution. This change aligns with AWS SDK's evolution but requires attention:
- Verify if the codebase uses any of these deprecated types
- Plan migration to service-specific endpoint resolvers
- Be aware that using deprecated types may prevent access to new endpoint-related features
Run this script to check for usage of deprecated types:
Also applies to: 131-132, 170-177, 184-185, 200-201, 208-209
Below is an improved verification script that avoids the earlier regex errors. Please run it and share the results so we can confirm whether any deprecated endpoint types (such as those using aws.Endpoint, custom implementations of ResolveEndpoint methods, or usage of EndpointResolverWithOptions) are being used in the codebase.
Action: Run the improved verification script below and manually verify its output.
After running this script:
- If the output shows that any deprecated types or methods are being used, plan a migration to the newer, service-specific endpoint resolvers.
- If no results are returned, then it appears that your codebase does not use these deprecated types.
Please verify the results manually and let me know if further clarification is needed.
Below is a follow-up verification script to inspect the internal usage (specifically in pkg/us3/s3.go) of deprecated endpoint types. Please run it and share the results, so we can concretely assess if any non‑vendor code relies on these types and plan necessary migration steps.
Once you run this script, please share the output so we can finalize our review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of deprecated endpoint types
# Check for Endpoint struct usage
echo "Checking for Endpoint struct usage..."
rg -l "type .+ struct \{[^}]*Endpoint\s+aws\.Endpoint[^}]*\}"
# Check for EndpointResolver interface implementation
echo "Checking for EndpointResolver interface implementation..."
rg -l "type .+ struct.*\n.*\n.*ResolveEndpoint\(service, region string\) \(aws\.Endpoint, error\)"
# Check for EndpointResolverWithOptions usage
echo "Checking for EndpointResolverWithOptions usage..."
rg -l "ResolveEndpoint\(.*string,.*string,.*\.\.\.interface{}\) \(aws\.Endpoint, error\)"
Length of output: 984
Script:
#!/bin/bash
# Improved verification for deprecated endpoint types usage
echo "1. Checking for deprecated 'Endpoint' struct usage (structs that contain 'aws.Endpoint'):"
rg -U -l "type\s+\w+\s+struct\s+{[^}]*\bEndpoint\s+aws\.Endpoint\b" .
echo "--------------------------------------------"
echo "2. Checking for functions implementing 'ResolveEndpoint(service, region string) (aws.Endpoint, error)':"
# This pattern looks for a method with signature matching ResolveEndpoint with service and region parameters.
rg -U -l "func\s+\(.*\)\s+ResolveEndpoint\(service,\s*region\s+string\)\s+\(aws\.Endpoint,\s*error\)" .
echo "--------------------------------------------"
echo "3. Searching for usage of 'EndpointResolverWithOptions':"
rg -l "EndpointResolverWithOptions" .
Length of output: 2502
Script:
#!/bin/bash
# Inspecting internal usage of deprecated endpoint types in pkg/us3/s3.go
echo "Inspecting pkg/us3/s3.go for occurrences of 'EndpointResolverWithOptions':"
rg -n "EndpointResolverWithOptions" pkg/us3/s3.go
echo "--------------------------------------------"
echo "Inspecting pkg/us3/s3.go for occurrences of 'aws.Endpoint':"
rg -n "aws\.Endpoint" pkg/us3/s3.go
Length of output: 724
| _, span := tracing.StartSpan(ctx, "ComputePayloadSHA256") | ||
| defer span.End() |
There was a problem hiding this comment.
Fix double span ending.
The tracing span is ended twice: once at line 166 and again at line 193. This could lead to undefined behavior.
Apply this diff to fix the double span ending:
_, span := tracing.StartSpan(ctx, "ComputePayloadSHA256")
- defer span.End()
// ... rest of the code ...
span.End()Also applies to: 193-193
| func NewClientOnlyConnectorRunner(ctx context.Context, c types.ConnectorClient, opts ...Option) (*connectorRunner, error) { | ||
| runner := &connectorRunner{} | ||
| cfg := &runnerConfig{} | ||
|
|
||
| for _, o := range opts { | ||
| err := o(ctx, cfg) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| var wrapperOpts []connector.Option | ||
| wrapperOpts = append(wrapperOpts, connector.WithRateLimiterConfig(cfg.rlCfg)) | ||
|
|
||
| for _, d := range cfg.rlDescriptors { | ||
| wrapperOpts = append(wrapperOpts, connector.WithRateLimitDescriptor(d)) | ||
| } | ||
|
|
||
| if cfg.provisioningEnabled { | ||
| wrapperOpts = append(wrapperOpts, connector.WithProvisioningEnabled()) | ||
| } | ||
|
|
||
| if cfg.ticketingEnabled { | ||
| wrapperOpts = append(wrapperOpts, connector.WithTicketingEnabled()) | ||
| } | ||
|
|
||
| if cfg.skipFullSync { | ||
| wrapperOpts = append(wrapperOpts, connector.WithFullSyncDisabled()) | ||
| } | ||
|
|
||
| cw, err := connector.NewClientOnlyWrapper(ctx, c, wrapperOpts...) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| runner.cw = cw | ||
|
|
||
| if cfg.onDemand { | ||
| if cfg.c1zPath == "" && cfg.eventFeedConfig == nil && cfg.createTicketConfig == nil && cfg.listTicketSchemasConfig == nil && cfg.getTicketConfig == nil && cfg.bulkCreateTicketConfig == nil { | ||
| return nil, errors.New("c1zPath must be set when in on-demand mode") | ||
| } | ||
|
|
||
| var tm tasks.Manager | ||
| switch { | ||
| case cfg.grantConfig != nil: | ||
| tm = local.NewGranter( | ||
| ctx, | ||
| cfg.c1zPath, | ||
| cfg.grantConfig.entitlementID, | ||
| cfg.grantConfig.principalID, | ||
| cfg.grantConfig.principalType, | ||
| ) | ||
|
|
||
| case cfg.revokeConfig != nil: | ||
| tm = local.NewRevoker(ctx, cfg.c1zPath, cfg.revokeConfig.grantID) | ||
|
|
||
| case cfg.createAccountConfig != nil: | ||
| tm = local.NewCreateAccountManager(ctx, cfg.c1zPath, cfg.createAccountConfig.login, cfg.createAccountConfig.email, cfg.createAccountConfig.profile) | ||
|
|
||
| case cfg.deleteResourceConfig != nil: | ||
| tm = local.NewResourceDeleter(ctx, cfg.c1zPath, cfg.deleteResourceConfig.resourceId, cfg.deleteResourceConfig.resourceType) | ||
|
|
||
| case cfg.rotateCredentialsConfig != nil: | ||
| tm = local.NewCredentialRotator(ctx, cfg.c1zPath, cfg.rotateCredentialsConfig.resourceId, cfg.rotateCredentialsConfig.resourceType) | ||
|
|
||
| case cfg.eventFeedConfig != nil: | ||
| tm = local.NewEventFeed(ctx) | ||
| case cfg.createTicketConfig != nil: | ||
| tm = local.NewTicket(ctx, cfg.createTicketConfig.templatePath) | ||
| case cfg.listTicketSchemasConfig != nil: | ||
| tm = local.NewListTicketSchema(ctx) | ||
| case cfg.getTicketConfig != nil: | ||
| tm = local.NewGetTicket(ctx, cfg.getTicketConfig.ticketID) | ||
| case cfg.bulkCreateTicketConfig != nil: | ||
| tm = local.NewBulkTicket(ctx, cfg.bulkCreateTicketConfig.templatePath) | ||
| default: | ||
| tm, err = local.NewSyncer(ctx, cfg.c1zPath, local.WithTmpDir(cfg.tempDir)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| runner.tasks = tm | ||
|
|
||
| runner.oneShot = true | ||
| return runner, nil | ||
| } | ||
|
|
||
| tm, err := c1api.NewC1TaskManager(ctx, cfg.clientID, cfg.clientSecret, cfg.tempDir, cfg.skipFullSync) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| runner.tasks = tm | ||
|
|
||
| return runner, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to reduce code duplication.
The NewClientOnlyConnectorRunner function contains significant code duplication with NewConnectorRunner. Consider refactoring to:
- Extract common configuration logic into a shared function
- Extract task manager initialization into a shared function
- Use a factory pattern for creating different types of runners
Here's a suggested approach:
// Extract common configuration logic
func configureRunner(ctx context.Context, cfg *runnerConfig, opts ...Option) error {
for _, o := range opts {
if err := o(ctx, cfg); err != nil {
return err
}
}
return nil
}
// Extract wrapper options building
func buildWrapperOptions(cfg *runnerConfig) []connector.Option {
opts := []connector.Option{
connector.WithRateLimiterConfig(cfg.rlCfg),
}
for _, d := range cfg.rlDescriptors {
opts = append(opts, connector.WithRateLimitDescriptor(d))
}
if cfg.provisioningEnabled {
opts = append(opts, connector.WithProvisioningEnabled())
}
if cfg.ticketingEnabled {
opts = append(opts, connector.WithTicketingEnabled())
}
if cfg.skipFullSync {
opts = append(opts, connector.WithFullSyncDisabled())
}
return opts
}
// Extract task manager creation
func createTaskManager(ctx context.Context, cfg *runnerConfig) (tasks.Manager, error) {
if cfg.onDemand {
if cfg.c1zPath == "" && cfg.eventFeedConfig == nil && cfg.createTicketConfig == nil &&
cfg.listTicketSchemasConfig == nil && cfg.getTicketConfig == nil &&
cfg.bulkCreateTicketConfig == nil {
return nil, errors.New("c1zPath must be set when in on-demand mode")
}
switch {
case cfg.grantConfig != nil:
return local.NewGranter(
ctx,
cfg.c1zPath,
cfg.grantConfig.entitlementID,
cfg.grantConfig.principalID,
cfg.grantConfig.principalType,
), nil
// ... other cases ...
}
}
return c1api.NewC1TaskManager(ctx, cfg.clientID, cfg.clientSecret, cfg.tempDir, cfg.skipFullSync)
}
// Refactored runner creation functions
func NewClientOnlyConnectorRunner(ctx context.Context, c types.ConnectorClient, opts ...Option) (*connectorRunner, error) {
runner := &connectorRunner{}
cfg := &runnerConfig{}
if err := configureRunner(ctx, cfg, opts...); err != nil {
return nil, err
}
cw, err := connector.NewClientOnlyWrapper(ctx, c, buildWrapperOptions(cfg)...)
if err != nil {
return nil, err
}
runner.cw = cw
tm, err := createTaskManager(ctx, cfg)
if err != nil {
return nil, err
}
runner.tasks = tm
runner.oneShot = cfg.onDemand
return runner, nil
}
func NewConnectorRunner(ctx context.Context, c types.ConnectorServer, opts ...Option) (*connectorRunner, error) {
runner := &connectorRunner{}
cfg := &runnerConfig{}
if err := configureRunner(ctx, cfg, opts...); err != nil {
return nil, err
}
cw, err := connector.NewWrapper(ctx, c, buildWrapperOptions(cfg)...)
if err != nil {
return nil, err
}
runner.cw = cw
tm, err := createTaskManager(ctx, cfg)
if err != nil {
return nil, err
}
runner.tasks = tm
runner.oneShot = cfg.onDemand
return runner, nil
}| @@ -107,3 +113,5 @@ require ( | |||
| modernc.org/memory v1.8.2 // indirect | |||
| modernc.org/sqlite v1.34.4 // indirect | |||
| ) | |||
|
|
|||
| replace github.com/ductone/c1-lambda => ../../conductorone/c1-lambda | |||
There was a problem hiding this comment.
Local Module Replacement Directive
The replace directive for github.com/ductone/c1-lambda is now pointing to a local path (../../conductorone/c1-lambda), which is flagged by static analysis as disallowed. Local replacements may violate project policies for module resolution. Please remove or update this directive to comply with the guidelines.
🧰 Tools
🪛 golangci-lint (1.62.2)
117-117: local replacement are not allowed: github.com/ductone/c1-lambda
(gomoddirectives)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
pkg/cli/lambda_command.go (5)
54-57: Enhance error context.Add more context to the error for better debugging.
- return nil, err + return nil, fmt.Errorf("failed to load AWS config: %w", err)
77-240: Consider breaking down the command function.The function is quite long and handles multiple responsibilities. Consider extracting the mode-specific logic into separate functions for better maintainability.
226-229: Simplify error handling.The error is already being logged, so you can return it directly.
- if err != nil { - l.Error("error creating connector runner", zap.Error(err)) - return err - } + if err != nil { + l.Error("error creating connector runner", zap.Error(err)) + return fmt.Errorf("failed to create connector runner: %w", err) + }
269-272: Enhance error context.Add more context to the error for better debugging.
- if err != nil { - return err - } + if err != nil { + return fmt.Errorf("failed to create Lambda connector client: %w", err) + }
380-384: Address FIXME comments.The FIXME comments indicate missing or uncertain functionality regarding rate limiting and feature flags. These should be addressed before the code is finalized.
Would you like me to help implement the rate limiter or propose a configuration structure for the provisioning and ticketing flags?
pkg/cli/commands_test.go (3)
12-17: Fix comment style.Add a period at the end of the comment to follow Go style guidelines.
-// Example struct for configuration +// Example struct for configuration.🧰 Tools
🪛 golangci-lint (1.62.2)
12-12: Comment should end in a period
(godot)
19-19: Fix comment style.Add a period at the end of the comment to follow Go style guidelines.
-// Generic function to unmarshal Viper configuration into a struct of type T +// Generic function to unmarshal Viper configuration into a struct of type T.🧰 Tools
🪛 golangci-lint (1.62.2)
19-19: Comment should end in a period
(godot)
20-58: Consider improving error handling and type constraints.The function could benefit from the following improvements:
- Use type constraints to enforce struct types at compile time.
- Provide more descriptive error messages.
Here's a suggested refactor:
// T must be a struct or a pointer to a struct. func makeGenericConfiguration[T any](v *viper.Viper) (T, error) { var config T if _, ok := any(config).(*viper.Viper); ok { return any(v).(T), nil } tType := reflect.TypeOf(config) isPointer := false if tType.Kind() == reflect.Ptr { tType = tType.Elem() isPointer = true } if tType.Kind() != reflect.Struct { return config, fmt.Errorf("expected T to be a struct or pointer to struct, got %s (isPointer=%v)", tType.Kind(), isPointer) } if !isPointer { ptr := reflect.New(tType) if err := v.Unmarshal(ptr.Interface()); err != nil { return config, fmt.Errorf("failed to unmarshal config into %s: %w", tType.Name(), err) } return ptr.Elem().Interface().(T), nil } if err := v.Unmarshal(&config); err != nil { return config, fmt.Errorf("failed to unmarshal config into %s: %w", tType.Name(), err) } return config, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/cli/commands_test.go(1 hunks)pkg/cli/lambda_command.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
pkg/cli/commands_test.go
74-74: use of fmt.Printf forbidden by pattern ^(fmt\.Print(|f|ln)|print|println)$
(forbidigo)
12-12: Comment should end in a period
(godot)
19-19: Comment should end in a period
(godot)
pkg/cli/lambda_command.go
341-341: use of fmt.Println forbidden by pattern ^(fmt\.Print(|f|ln)|print|println)$
(forbidigo)
48-48: non-wrapping format verb for fmt.Errorf. Use %w to format errors
(errorlint)
🔇 Additional comments (3)
pkg/cli/lambda_command.go (2)
48-48: Use wrapping verb%wfor error.Replace
%vwith%wto enable error unwrapping in Go.- return nil, fmt.Errorf("failed to parse endpoint: %v", err) + return nil, fmt.Errorf("failed to parse endpoint: %w", err)🧰 Tools
🪛 golangci-lint (1.62.2)
48-48: non-wrapping format verb for fmt.Errorf. Use
%wto format errors(errorlint)
341-341: Avoid usingfmt.Printlnfor logging.Replace
fmt.Printlnwith structured logging using zap.- fmt.Println("Received config: ", spew.Sdump(config)) + l := ctxzap.Extract(ctx) + l.Info("Received config", zap.String("debug", spew.Sdump(config)))🧰 Tools
🪛 golangci-lint (1.62.2)
341-341: use of
fmt.Printlnforbidden by pattern^(fmt\.Print(|f|ln)|print|println)$(forbidigo)
pkg/cli/commands_test.go (1)
1-76: Verify experimental changes and merge restrictions.This PR is marked as experimental with a "do not merge" directive. Please clarify:
- The experimental aspects of these configuration changes.
- The timeline for completing the experiment.
- The criteria for determining if the experiment is successful.
🧰 Tools
🪛 golangci-lint (1.62.2)
74-74: use of
fmt.Printfforbidden by pattern^(fmt\.Print(|f|ln)|print|println)$(forbidigo)
12-12: Comment should end in a period
(godot)
19-19: Comment should end in a period
(godot)
| func TestGenerics(t *testing.T) { | ||
| // Initialize Viper | ||
| v := viper.New() | ||
| v.SetConfigType("yaml") | ||
|
|
||
| // Simulate setting values in Viper (normally read from a file) | ||
| v.Set("port", 8080) | ||
| v.Set("host", "localhost") | ||
| v.Set("with-dash", "foobar") | ||
|
|
||
| // Use the generic function to load config | ||
| config, err := makeGenericConfiguration[AppConfig](v) | ||
| require.NoError(t, err) | ||
|
|
||
| fmt.Printf("Loaded Config: %+v\n", config) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage and fix linting issues.
The test function could be improved in several ways:
- Replace
fmt.Printfwitht.Logfto comply with linting rules. - Add assertions to validate each field of the loaded configuration.
- Add test cases for error scenarios.
Here's a suggested refactor:
func TestGenerics(t *testing.T) {
tests := []struct {
name string
input map[string]interface{}
want AppConfig
wantErr bool
}{
{
name: "valid config",
input: map[string]interface{}{
"port": 8080,
"host": "localhost",
"with-dash": "foobar",
},
want: AppConfig{
Port: 8080,
Host: "localhost",
WithDash: "foobar",
},
wantErr: false,
},
// Add more test cases for error scenarios
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
v := viper.New()
v.SetConfigType("yaml")
for k, val := range tt.input {
v.Set(k, val)
}
got, err := makeGenericConfiguration[AppConfig](v)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tt.want.Port, got.Port)
require.Equal(t, tt.want.Host, got.Host)
require.Equal(t, tt.want.WithDash, got.WithDash)
t.Logf("Loaded Config: %+v", got)
})
}
}🧰 Tools
🪛 golangci-lint (1.62.2)
74-74: use of fmt.Printf forbidden by pattern ^(fmt\.Print(|f|ln)|print|println)$
(forbidigo)
do not merge
Questions:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Chores