Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add HTTP2 support for http.Client and Vald HTTP Server #2572

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

kpango
Copy link
Collaborator

@kpango kpango commented Aug 6, 2024

Description

  • New Features

    • Introduced detailed HTTP/2 configuration options across multiple services, enhancing performance and customization for users.
    • Added a new Docker image build workflow for the buildkit-syft-scanner, improving build cache capabilities.
  • Version Updates

    • Updated Vald version from v1.7.12 to v1.7.13.
    • Updated Go version from v1.22.5 to v1.22.6.
  • Bug Fixes

    • Enhanced error handling and transport configuration in the HTTP client, improving network communication reliability.
    • Helm CI Workflow failed, due to the host.docker.internal option not found in workflow.
  • Documentation

    • Revised issue and pull request templates to reflect the latest versions of Vald and Go, ensuring users are aware of current software dependencies.

Related Issue

Versions

  • Vald Version: v1.7.12
  • Go Version: v1.22.5
  • Rust Version: v1.80.0
  • Docker Version: v27.1.1
  • Kubernetes Version: v1.30.3
  • Helm Version: v3.15.3
  • NGT Version: v2.2.4
  • Faiss Version: v1.8.0

Checklist

Special notes for your reviewer

Summary by CodeRabbit

  • New Features

    • Introduced detailed HTTP/2 configuration options across multiple services, enhancing performance and customization for users.
    • Added a new Docker image build workflow for the buildkit-syft-scanner, improving build automation capabilities.
  • Version Updates

    • Updated Vald version from v1.7.12 to v1.7.13.
    • Updated Go version from v1.22.5 to v1.22.6.
  • Bug Fixes

    • Enhanced error handling and transport configuration in the HTTP client, improving network communication reliability.
  • Documentation

    • Revised issue and pull request templates to reflect the latest versions of Vald and Go, ensuring users are aware of current software dependencies.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Aug 6, 2024

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Aug 6, 2024

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copy link
Contributor

coderabbitai bot commented Aug 6, 2024

Walkthrough

Walkthrough

This update introduces enhancements across various components, primarily focusing on version increments for Vald and Go, alongside significant additions to HTTP/2 configurations in multiple YAML files. The modifications improve bug report and pull request templates, streamline Docker build workflows, and expand server configuration capabilities for HTTP/2. Overall, the changes aim to enhance performance, maintainability, and adaptability of the system.

Changes

Files Change Summary
.github/ISSUE_TEMPLATE/*.md, .github/PULL_REQUEST_TEMPLATE.md Updated Vald and Go versions from v1.7.12 to v1.7.13 and v1.22.5 to v1.22.6, respectively, in the bug report and pull request templates.
.github/workflows/*.yaml Introduced a new workflow for building a Docker image for the buildkit-syft-scanner and added HTTP/2 configuration options to the Helm workflow.
Makefile, Makefile.d/docker.mk Added a new image variable for the BuildKit scanner and implemented a target to remove empty files, enhancing file management.
charts/vald-helm-operator/crds/valdrelease.yaml, charts/vald/values.yaml Added a new http2 property with various configuration options to support HTTP/2 in the release and values files, enhancing their configurability.
dockers/*.Dockerfile Reorganized environment variable declarations for clarity and introduced a new Dockerfile for the buildkit-syft-scanner.
example/client/go.mod, go.mod Updated several dependencies in Go projects to their latest versions, ensuring performance improvements and bug fixes.
internal/config/server.go, internal/net/http/client/client.go Enhanced server configuration to include new HTTP/2 settings, including TLS support and improved transport configuration.
k8s/**/*.yaml Added HTTP/2 configurations to multiple Kubernetes ConfigMaps for ports 3000, 3001, and 6060, providing extensive HTTP/2 settings while maintaining backward compatibility.
versions/*.VERSION Incremented version numbers for Go, Vald, and various actions, indicating minor updates and potential bug fixes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant System
    participant Docker
    participant Kubernetes

    User->>System: Submit Bug Report
    System->>Docker: Build Docker Image
    Docker->>Kubernetes: Deploy Configuration
    Kubernetes->>System: Update Configuration with HTTP/2
    System->>User: Acknowledge Bug Fixes
Loading
sequenceDiagram
    participant Client
    participant Server

    Client->>Server: HTTP Request
    alt HTTP/2 Enabled
        Server->>Server: Handle HTTP/2
        Server->>Client: HTTP/2 Response
    else HTTP/2 Disabled
        Server->>Client: HTTP/1.1 Response
    end
Loading

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 6.66667% with 84 lines in your changes missing coverage. Please review.

Project coverage is 17.39%. Comparing base (154d261) to head (1c30a54).
Report is 2 commits behind head on main.

Files Patch % Lines
internal/servers/server/option.go 0.00% 59 Missing ⚠️
internal/config/server.go 0.00% 11 Missing and 1 partial ⚠️
internal/servers/server/server.go 8.33% 11 Missing ⚠️
internal/net/http/client/client.go 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2572      +/-   ##
==========================================
- Coverage   17.39%   17.39%   -0.01%     
==========================================
  Files         566      566              
  Lines       69059    69143      +84     
==========================================
+ Hits        12015    12026      +11     
- Misses      56247    56319      +72     
- Partials      797      798       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (10)
internal/config/server.go (6)

32-33: Add a comment for the TLS field.

Consider adding a comment to explain the TLS field for better readability and maintainability.

+	// TLS represents the server TLS configuration.
	TLS *TLS `json:"tls" yaml:"tls"`

35-36: Add a comment for the FullShutdownDuration field.

Consider adding a comment to explain the FullShutdownDuration field for better readability and maintainability.

+	// FullShutdownDuration represents the total duration for a complete shutdown.
	FullShutdownDuration string `json:"full_shutdown_duration" yaml:"full_shutdown_duration"`

66-66: Add a comment for the Port field.

Consider adding a comment to explain the Port field for better readability and maintainability.

+	// Port represents the server port number.
	Port uint16 `json:"port,omitempty" yaml:"port"`

71-71: Add a comment for the HTTP2 field.

Consider adding a comment to explain the HTTP2 field for better readability and maintainability.

+	// HTTP2 represents the configuration for HTTP/2.
	HTTP2 *HTTP2 `json:"http2" yaml:"http2"`

81-91: Add comments for the fields in the HTTP2 struct.

Consider adding comments to explain each field in the HTTP2 struct for better readability and maintainability.

type HTTP2 struct {
+	// HandlerLimit specifies the maximum number of handlers.
	HandlerLimit int `json:"handler_limit,omitempty" yaml:"handler_limit"`
+	// Enabled indicates whether HTTP/2 is enabled.
	Enabled bool `json:"enabled,omitempty" yaml:"enabled"`
+	// PermitProhibitedCipherSuites allows the use of prohibited cipher suites.
	PermitProhibitedCipherSuites bool `json:"permit_prohibited_cipher_suites,omitempty" yaml:"permit_prohibited_cipher_suites"`
+	// MaxUploadBufferPerConnection specifies the maximum upload buffer size per connection.
	MaxUploadBufferPerConnection int32 `json:"max_upload_buffer_per_connection,omitempty" yaml:"max_upload_buffer_per_connection"`
+	// MaxUploadBufferPerStream specifies the maximum upload buffer size per stream.
	MaxUploadBufferPerStream int32 `json:"max_upload_buffer_per_stream,omitempty" yaml:"max_upload_buffer_per_stream"`
+	// MaxConcurrentStreams specifies the maximum number of concurrent streams.
	MaxConcurrentStreams uint32 `json:"max_concurrent_streams,omitempty" yaml:"max_concurrent_streams"`
+	// MaxDecoderHeaderTableSize specifies the maximum size of the decoder header table.
	MaxDecoderHeaderTableSize uint32 `json:"max_decoder_header_table_size,omitempty" yaml:"max_decoder_header_table_size"`
+	// MaxEncoderHeaderTableSize specifies the maximum size of the encoder header table.
	MaxEncoderHeaderTableSize uint32 `json:"max_encoder_header_table_size,omitempty" yaml:"max_encoder_header_table_size"`
+	// MaxReadFrameSize specifies the maximum frame size that can be read.
	MaxReadFrameSize uint32 `json:"max_read_frame_size,omitempty" yaml:"max_read_frame_size"`
}

95-99: Add comments for the new fields in the GRPC struct.

Consider adding comments to explain each new field in the GRPC struct for better readability and maintainability.

type GRPC struct {
+	// Keepalive represents the gRPC keepalive configuration.
	Keepalive *GRPCKeepalive `json:"keepalive,omitempty" yaml:"keepalive"`
+	// ConnectionTimeout specifies the connection timeout duration.
	ConnectionTimeout string `json:"connection_timeout,omitempty" yaml:"connection_timeout"`
+	// Interceptors lists the gRPC interceptors to be used.
	Interceptors []string `json:"interceptors,omitempty" yaml:"interceptors"`
+	// EnableReflection indicates whether gRPC reflection is enabled.
	EnableReflection bool `json:"enable_reflection,omitempty" yaml:"enable_reflection"`
+	// EnableAdmin indicates whether the gRPC admin interface is enabled.
	EnableAdmin bool `json:"enable_admin,omitempty" yaml:"enable_admin"`
}
charts/vald/values.yaml (4)

97-125: Add documentation for new HTTP/2 settings.

The new HTTP/2 settings should be documented to ensure users understand the purpose and usage of each parameter.

# @schema {"name": "defaults.server_config.servers.rest.server.http.http2", "type": "object", "anchor": "http2_server_config"}
http2:
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.enabled", "type": "boolean"}
  # defaults.server_config.servers.rest.server.http.http2.enabled -- HTTP2 server enabled
  enabled: false
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.handler_limit", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.handler_limit -- Limits the number of http.Handler ServeHTTP goroutines which may run at a time over all connections. Negative or zero no limit.
  handler_limit: 0
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.permit_prohibited_cipher_suites", "type": "boolean"}
  # defaults.server_config.servers.rest.server.http.http2.permit_prohibited_cipher_suites -- if true, permits the use of cipher suites prohibited by the HTTP/2 spec.
  permit_prohibited_cipher_suites: true
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_upload_buffer_per_connection", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_upload_buffer_per_connection -- The size of the initial flow control window for each connections.
  max_upload_buffer_per_connection: 0
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_upload_buffer_per_stream", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_upload_buffer_per_stream -- The size of the initial flow control window for each streams.
  max_upload_buffer_per_stream: 0
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_concurrent_streams", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_concurrent_streams -- The number of concurrent streams that each client may have open at a time.
  max_concurrent_streams: 0
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_decoder_header_table_size", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_decoder_header_table_size -- Informs the remote endpoint of the maximum size of the header compression table used to decode header blocks, in octets. If zero, the default value of 4096 is used.
  max_decoder_header_table_size: 4096
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_encoder_header_table_size", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_encoder_header_table_size -- An upper limit for the header compression table used for encoding request headers.
  max_encoder_header_table_size: 4096
  # @schema {"name": "defaults.server_config.servers.rest.server.http.http2.max_read_frame_size", "type": "integer"}
  # defaults.server_config.servers.rest.server.http.http2.max_read_frame_size -- The largest frame this server is willing to read.
  max_read_frame_size: 0

388-407: Add documentation for new HTTP/2 settings.

The new HTTP/2 settings should be documented to ensure users understand the purpose and usage of each parameter.

# @schema {"name": "defaults.server_config.healths.liveness.server.http.http2", "type": "object", "alias": "http2_server_config"}
http2:
  # defaults.server_config.healths.liveness.server.http.http2.enabled -- HTTP2 server enabled
  enabled: false
  # defaults.server_config.healths.liveness.server.http.http2.handler_limit -- Limits the number of http.Handler ServeHTTP goroutines which may run at a time over all connections. Negative or zero no limit.
  handler_limit: 0
  # defaults.server_config.healths.liveness.server.http.http2.permit_prohibited_cipher_suites -- if true, permits the use of cipher suites prohibited by the HTTP/2 spec.
  permit_prohibited_cipher_suites: true
  # defaults.server_config.healths.liveness.server.http.http2.max_upload_buffer_per_connection -- The size of the initial flow control window for each connections.
  max_upload_buffer_per_connection: 0
  # defaults.server_config.healths.liveness.server.http.http2.max_upload_buffer_per_stream -- The size of the initial flow control window for each streams.
  max_upload_buffer_per_stream: 0
  # defaults.server_config.healths.liveness.server.http.http2.max_concurrent_streams -- The number of concurrent streams that each client may have open at a time.
  max_concurrent_streams: 0
  # defaults.server_config.healths.liveness.server.http.http2.max_decoder_header_table_size -- Informs the remote endpoint of the maximum size of the header compression table used to decode header blocks, in octets. If zero, the default value of 4096 is used.
  max_decoder_header_table_size: 4096
  # defaults.server_config.healths.liveness.server.http.http2.max_encoder_header_table_size -- An upper limit for the header compression table used for encoding request headers.
  max_encoder_header_table_size: 4096
  # defaults.server_config.healths.liveness.server.http.http2.max_read_frame_size -- The largest frame this server is willing to read.
  max_read_frame_size: 0

508-526: Add documentation for new HTTP/2 settings.

The new HTTP/2 settings should be documented to ensure users understand the purpose and usage of each parameter.

# @schema {"name": "defaults.server_config.healths.readiness.server.http.http2", "type": "object", "alias": "http2_server_config"}
http2:
  # defaults.server_config.healths.readiness.server.http.http2.enabled -- HTTP2 server enabled
  enabled: false
  # defaults.server_config.healths.readiness.server.http.http2.handler_limit -- Limits the number of http.Handler ServeHTTP goroutines which may run at a time over all connections. Negative or zero no limit.
  handler_limit: 0
  # defaults.server_config.healths.readiness.server.http.http2.permit_prohibited_cipher_suites -- if true, permits the use of cipher suites prohibited by the HTTP/2 spec.
  permit_prohibited_cipher_suites: true
  # defaults.server_config.healths.readiness.server.http.http2.max_upload_buffer_per_connection -- The size of the initial flow control window for each connections.
  max_upload_buffer_per_connection: 0
  # defaults.server_config.healths.readiness.server.http.http2.max_upload_buffer_per_stream -- The size of the initial flow control window for each streams.
  max_upload_buffer_per_stream: 0
  # defaults.server_config.healths.readiness.server.http.http2.max_concurrent_streams -- The number of concurrent streams that each client may have open at a time.
  max_concurrent_streams: 0
  # defaults.server_config.healths.readiness.server.http.http2.max_decoder_header_table_size -- Informs the remote endpoint of the maximum size of the header compression table used to decode header blocks, in octets. If zero, the default value of 4096 is used.
  max_decoder_header_table_size: 4096
  # defaults.server_config.healths.readiness.server.http.http2.max_encoder_header_table_size -- An upper limit for the header compression table used for encoding request headers.
  max_encoder_header_table_size: 4096
  # defaults.server_config.healths.readiness.server.http.http2.max_read_frame_size -- The largest frame this server is willing to read.
  max_read_frame_size: 0

601-619: Add documentation for new HTTP/2 settings.

The new HTTP/2 settings should be documented to ensure users understand the purpose and usage of each parameter.

# @schema {"name": "defaults.server_config.metrics.pprof.server.http.http2", "type": "object", "alias": "http2_server_config"}
http2:
  # defaults.server_config.metrics.pprof.server.http.http2.enabled -- HTTP2 server enabled
  enabled: false
  # defaults.server_config.metrics.pprof.server.http.http2.handler_limit -- Limits the number of http.Handler ServeHTTP goroutines which may run at a time over all connections. Negative or zero no limit.
  handler_limit: 0
  # defaults.server_config.metrics.pprof.server.http.http2.permit_prohibited_cipher_suites -- if true, permits the use of cipher suites prohibited by the HTTP/2 spec.
  permit_prohibited_cipher_suites: true
  # defaults.server_config.metrics.pprof.server.http.http2.max_upload_buffer_per_connection -- The size of the initial flow control window for each connections.
  max_upload_buffer_per_connection: 0
  # defaults.server_config.metrics.pprof.server.http.http2.max_upload_buffer_per_stream -- The size of the initial flow control window for each streams.
  max_upload_buffer_per_stream: 0
  # defaults.server_config.metrics.pprof.server.http.http2.max_concurrent_streams -- The number of concurrent streams that each client may have open at a time.
  max_concurrent_streams: 0
  # defaults.server_config.metrics.pprof.server.http.http2.max_decoder_header_table_size -- Informs the remote endpoint of the maximum size of the header compression table used to decode header blocks, in octets. If zero, the default value of 4096 is used.
  max_decoder_header_table_size: 4096
  # defaults.server_config.metrics.pprof.server.http.http2.max_encoder_header_table_size -- An upper limit for the header compression table used for encoding request headers.
  max_encoder_header_table_size: 4096
  # defaults.server_config.metrics.pprof.server.http.http2.max_read_frame_size -- The largest frame this server is willing to read.
  max_read_frame_size: 0
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 74c85dc and eeadafc.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (34)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • go.mod (4 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/VALD_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (10)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/agent/core/agent/Dockerfile
  • dockers/buildkit/syft/scanner/Dockerfile
  • dockers/ci/base/Dockerfile
  • example/client/go.mod
  • versions/GO_VERSION
  • versions/VALD_VERSION
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional comments not posted (116)
.github/workflows/dockers-buildkit-syft-scanner-image.yaml (1)

1-48: LGTM! The GitHub Actions workflow is well-structured.

The workflow follows best practices, including cron scheduling, branch and tag filters, and the reuse of a predefined workflow. The inclusion of secrets is also appropriate.

internal/net/http/client/client.go (2)

52-61: LGTM! The modifications improve transport configuration and error handling.

The use of http2.ConfigureTransports enhances the robustness of the HTTP client initialization. The error handling and the use of exponential backoff are well-implemented.


66-69: LGTM! The transport configuration is correctly applied.

The h2 transport is correctly passed to the exponential backoff mechanism, ensuring the client uses the proper transport settings.

.github/workflows/helm.yml (1)

38-38: LGTM! The new configuration option enhances container networking capabilities.

The addition of the --add-host host.docker.internal:host-gateway option facilitates communication between the container and the host, improving the container's networking capabilities.

k8s/index/operator/deployment.yaml (1)

49-49: Update reflects latest configuration changes.

The update to the checksum/configmap annotation ensures that the deployment uses the latest configuration. This is a necessary change to maintain the application's operational integrity.

k8s/discoverer/deployment.yaml (1)

49-49: Update reflects latest configuration changes.

The update to the checksum/configmap annotation ensures that the deployment uses the latest configuration. This is a necessary change to maintain the application's operational integrity.

k8s/manager/index/deployment.yaml (1)

49-49: Update reflects latest configuration changes.

The update to the checksum/configmap annotation ensures that the deployment uses the latest configuration. This is a necessary change to maintain the application's operational integrity.

k8s/gateway/gateway/lb/deployment.yaml (1)

48-48: Verify the configuration changes.

The checksum value for the configmap has been updated. Ensure that the configuration changes referenced by this checksum are correct and that the deployment is refreshed to use the latest settings.

k8s/agent/ngt/configmap.yaml (3)

85-94: Verify the HTTP/2 configuration for port 3000.

The HTTP/2 configuration has been added for port 3000 with default values. Ensure that these settings are correct and meet the requirements.


120-129: Verify the HTTP/2 configuration for port 3001.

The HTTP/2 configuration has been added for port 3001 with default values. Ensure that these settings are correct and meet the requirements.


156-165: Verify the HTTP/2 configuration for port 6060.

The HTTP/2 configuration has been added for port 6060 with default values. Ensure that these settings are correct and meet the requirements.

k8s/discoverer/configmap.yaml (3)

85-94: Verify the HTTP/2 configuration for port 3000.

The HTTP/2 configuration has been added for port 3000 with default values. Ensure that these settings are correct and meet the requirements.


120-129: Verify the HTTP/2 configuration for port 3001.

The HTTP/2 configuration has been added for port 3001 with default values. Ensure that these settings are correct and meet the requirements.


156-165: Verify the HTTP/2 configuration for port 6060.

The HTTP/2 configuration has been added for port 6060 with default values. Ensure that these settings are correct and meet the requirements.

internal/servers/server/server.go (4)

43-43: Import for http2 looks good.

The addition of the http2 import is necessary for the new HTTP/2 functionality.


94-95: New fields for HTTP/2 support look good.

The addition of h2srv and enableH2 fields to the http struct is necessary for managing HTTP/2 server configuration and enablement.


190-204: HTTP/2 server configuration in the starter function looks good.

The logic to conditionally configure the HTTP/2 server, including setting the IdleTimeout and calling http2.ConfigureServer, is correct.


Line range hint 284-284: Ensure correct integration of HTTP/2 configuration in ListenAndServe.

The ListenAndServe function calls the starter function, which now includes HTTP/2 configuration. Ensure that this integration works as expected.

Makefile.d/docker.mk (3)

28-28: Addition of docker/build/buildkit-syft-scanner target looks good.

The inclusion of the buildkit-syft-scanner target in the docker/build target is necessary for the build process.


71-71: Updated build argument for SBOM looks good.

The update to use --attest type=sbom,generator=docker/buildkit-syft-scanner:edge enhances the SBOM generation process by specifying the generator and version.


223-233: New phony targets for buildkit-syft-scanner look good.

The addition of docker/name/buildkit-syft-scanner and docker/build/buildkit-syft-scanner targets facilitates the naming and building of the buildkit-syft-scanner image, enhancing the modularity and clarity of the build process.

k8s/gateway/gateway/lb/configmap.yaml (3)

85-94: HTTP/2 configuration block for port 3000 looks good.

The addition of the http2 configuration block for port 3000 includes various parameters to control the behavior of HTTP/2 connections. By default, HTTP/2 is disabled.


120-129: HTTP/2 configuration block for port 3001 looks good.

The addition of the http2 configuration block for port 3001 includes various parameters to control the behavior of HTTP/2 connections. By default, HTTP/2 is disabled.


156-165: HTTP/2 configuration block for port 6060 looks good.

The addition of the http2 configuration block for port 6060 includes various parameters to control the behavior of HTTP/2 connections. By default, HTTP/2 is disabled.

k8s/index/job/save/configmap.yaml (3)

85-94: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3000 looks good. Ensure that these settings are tested and verified for their impact on the system.


120-129: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3001 looks good. Ensure that these settings are tested and verified for their impact on the system.


156-165: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 6060 looks good. Ensure that these settings are tested and verified for their impact on the system.

k8s/index/job/creation/configmap.yaml (3)

85-94: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3000 looks good. Ensure that these settings are tested and verified for their impact on the system.


120-129: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3001 looks good. Ensure that these settings are tested and verified for their impact on the system.


156-165: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 6060 looks good. Ensure that these settings are tested and verified for their impact on the system.

k8s/manager/index/configmap.yaml (3)

85-94: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3000 looks good. Ensure that these settings are tested and verified for their impact on the system.


120-129: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 3001 looks good. Ensure that these settings are tested and verified for their impact on the system.


156-165: LGTM! Verify the impact of HTTP/2 settings.

The HTTP/2 configuration block for port 6060 looks good. Ensure that these settings are tested and verified for their impact on the system.

internal/config/server.go (1)

274-286: Ensure proper handling of HTTP/2 options.

The code correctly appends HTTP/2 options based on the Enabled status. Ensure that all necessary HTTP/2 options are included and properly handled.

k8s/index/operator/configmap.yaml (3)

28-28: LGTM! HTTP/2 configuration for health check servers is well-structured.

The settings are comprehensive and consistent with best practices for HTTP/2.


28-28: LGTM! HTTP/2 configuration for readiness servers is consistent and well-structured.

The settings ensure uniformity across the server configurations.


28-28: LGTM! HTTP/2 configuration for metrics servers is well-structured.

The settings are comprehensive and consistent with best practices for HTTP/2.

Makefile (2)

35-35: LGTM! Addition of BUILDKIT_SYFT_SCANNER_IMAGE variable.

The variable expands the set of images defined in the file, indicating the inclusion of a new component related to BuildKit functionality.


482-488: LGTM! Addition of remove/empty/file target.

The target enhances the file management capabilities of the Makefile, ensuring that unnecessary empty files are cleaned up as part of the build process.

go.mod (8)

309-309: Verify compatibility of golang.org/x/text update.

Ensure that the update from v0.16.0 to v0.17.0 does not introduce any breaking changes.


304-304: Verify compatibility of golang.org/x/net update.

Ensure that the update from v0.27.0 to v0.28.0 does not introduce any breaking changes.


297-297: Verify compatibility of golang.org/x/crypto update.

Ensure that the update from v0.25.0 to v0.26.0 does not introduce any breaking changes.


300-300: Verify compatibility of golang.org/x/image update.

Ensure that the update from v0.18.0 to v0.19.0 does not introduce any breaking changes.

Verification successful

No issues found with golang.org/x/image update.

The update from v0.18.0 to v0.19.0 does not impact the codebase as there are no import statements or function calls related to this dependency.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of `golang.org/x/image` update.

# Test: Search for the usage of `golang.org/x/image` in the codebase.
rg --type go -A 5 $'golang.org/x/image'

Length of output: 38


Script:

#!/bin/bash
# Description: Verify compatibility of `golang.org/x/image` update.

# Test: Search for import statements of `golang.org/x/image` in the codebase.
ast-grep --lang go --pattern 'import "golang.org/x/image"'

# Test: Search for function calls from `golang.org/x/image` in the codebase.
ast-grep --lang go --pattern 'golang.org/x/image.$_'

Length of output: 8544


308-308: Verify compatibility of golang.org/x/term update.

Ensure that the update from v0.22.0 to v0.23.0 does not introduce any breaking changes.

Verification successful

Verify compatibility of golang.org/x/term update.

Ensure that the update from v0.22.0 to v0.23.0 does not introduce any breaking changes. The initial search did not produce any output, so further verification is needed.


Verified compatibility of golang.org/x/term update.

The update from v0.22.0 to v0.23.0 includes only minor legal and dependency updates, with no functional changes. This should not introduce any breaking changes.

  • golang.org/x/term is listed in go.sum, indicating indirect usage.
  • Changes between versions are non-functional.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of `golang.org/x/term` update.

# Test: Search for the usage of `golang.org/x/term` in the codebase.
rg --type go -A 5 $'golang.org/x/term'

Length of output: 37


Script:

#!/bin/bash
# Description: Verify compatibility of `golang.org/x/term` update.

# Test 1: Search for the usage of `golang.org/x/term` in the codebase, including indirect references.
rg --type go -A 5 'golang.org/x/term'

# Test 2: Check the go.sum file to see if `golang.org/x/term` is listed, indicating it is being used indirectly.
rg 'golang.org/x/term' go.sum

# Test 3: Retrieve the changes between versions `v0.22.0` and `v0.23.0` of `golang.org/x/term`.
curl -s https://api.github.com/repos/golang/term/compare/v0.22.0...v0.23.0 | jq '.commits[] | {message: .commit.message, url: .html_url}'

Length of output: 1659


18-18: Verify compatibility of code.cloudfoundry.org/bytefmt update.

Ensure that the update from v0.0.0-20240804182054-0a63f33a903d to v0.0.0-20240806182212-6cf545ebdd6b does not introduce any breaking changes.


307-307: Verify compatibility of golang.org/x/sync update.

Ensure that the update from v0.7.0 to v0.8.0 does not introduce any breaking changes.


311-311: Verify compatibility of golang.org/x/tools update.

Ensure that the update from v0.23.0 to v0.24.0 does not introduce any breaking changes.

Verification successful

Verify compatibility of golang.org/x/tools update.

Ensure that the update from v0.23.0 to v0.24.0 does not introduce any breaking changes in the analysistest package.


Verify compatibility of golang.org/x/tools update.

Ensure that the update from v0.23.0 to v0.24.0 does not introduce any breaking changes in the analysistest package.


Verify compatibility of golang.org/x/tools update.

Ensure that the update from v0.23.0 to v0.24.0 does not introduce any breaking changes in the analysistest package.


The update from v0.23.0 to v0.24.0 for golang.org/x/tools is compatible.

The diff shows that new tests were added to the analysistest package, but no breaking changes to existing functionality were introduced.

  • No breaking changes were found in the analysistest package.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of `golang.org/x/tools` update.

# Test: Search for the usage of `golang.org/x/tools` in the codebase.
rg --type go -A 5 $'golang.org/x/tools'

Length of output: 393


Script:

#!/bin/bash
# Description: Check for breaking changes in `golang.org/x/tools` update.

# Fetch the changelog or release notes for `golang.org/x/tools`.
curl -s https://api.github.com/repos/golang/tools/releases | jq -r '.[] | select(.tag_name == "v0.24.0" or .tag_name == "v0.23.0") | .body'

Length of output: 141


Script:

#!/bin/bash
# Description: Check for breaking changes in `golang.org/x/tools` update by examining the changelog.

# Search for changelog file in the `golang.org/x/tools` repository.
fd -i changelog | xargs cat

Length of output: 244190


Script:

#!/bin/bash
# Description: Fetch the diff between `v0.23.0` and `v0.24.0` for `golang.org/x/tools/go/analysis/analysistest`.

# Clone the repository to get the diffs
git clone https://github.com/golang/tools.git
cd tools

# Fetch the diffs between the two versions for the specific package
git diff v0.23.0 v0.24.0 -- go/analysis/analysistest

Length of output: 2252

charts/vald-helm-operator/crds/valdrelease.yaml (24)

702-722: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


831-851: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


1041-1061: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


1546-1566: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


1663-1683: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


1792-1812: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


2002-2022: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


2481-2501: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


2598-2618: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


2727-2747: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


2937-2957: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


3432-3452: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


3549-3569: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


3678-3698: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


3888-3908: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


4887-4907: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


5004-5024: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


5133-5153: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


5343-5363: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


6315-6335: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


6432-6452: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


6561-6581: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


6771-6791: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.


585-605: LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure.

Verify that similar changes are consistently applied across the file.

Verification successful

LGTM! Ensure consistency across the file.

The added HTTP/2 configuration properties are well-defined and follow the correct schema structure. The review confirms that similar changes are consistently applied across the file.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of HTTP/2 configuration properties across the file.

# Test: Search for the `http2` property. Expect: Consistent structure and properties.
rg --type yaml -A 20 $'http2'

Length of output: 321357

k8s/operator/helm/crds/valdrelease.yaml (33)

702-722: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


831-851: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


1041-1061: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


1546-1566: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


1663-1683: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


1792-1812: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


2002-2022: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


2481-2501: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


2598-2618: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


2727-2747: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


2937-2957: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


3432-3452: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


3549-3569: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


3678-3698: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


3888-3908: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


4887-4907: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


5004-5024: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


5133-5153: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


5343-5363: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


6315-6335: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


6432-6452: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


6561-6581: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


6771-6791: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


7463-7483: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


7580-7600: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


7709-7729: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


7919-7939: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


8795-8815: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


8912-8932: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


9041-9061: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


9251-9271: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


9861-9881: LGTM!

The added HTTP/2 configuration properties are correctly structured and typed.


585-605: LGTM! Verify consistency across the file.

The added HTTP/2 configuration properties are correctly structured and typed.

Ensure that similar properties are consistently added across the file.

Verification successful

Verification Successful: HTTP/2 configuration properties are consistently added across the file and other related files.

The properties enabled, handler_limit, max_concurrent_streams, max_decoder_header_table_size, max_encoder_header_table_size, max_read_frame_size, max_upload_buffer_per_connection, max_upload_buffer_per_stream, and permit_prohibited_cipher_suites are consistently present.

  • Files verified include multiple occurrences in k8s/operator/helm/crds/valdrelease.yaml and other related files.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all HTTP/2 configuration properties are consistently added across the file.

# Test: Search for the HTTP/2 configuration properties. Expect: Consistent properties across the file.
rg --type yaml -A 20 $'http2:\n  type: object\n  properties:'

Length of output: 252


Script:

#!/bin/bash
# Description: Verify all HTTP/2 configuration properties are consistently added across the file.

# Test: Search for the HTTP/2 configuration properties. Expect: Consistent properties across the file.
rg --type yaml -U -A 20 $'http2:\n  type: object\n  properties:'

Length of output: 63


Script:

#!/bin/bash
# Description: Verify all HTTP/2 configuration properties are consistently added across the file.

# Test: Search for the HTTP/2 configuration properties. Expect: Consistent properties across the file.
rg --type yaml -U -A 30 'http2:'

Length of output: 438471

internal/servers/server/option.go (9)

279-284: LGTM!

The function correctly enables or disables HTTP/2 support.


286-296: LGTM!

The function correctly sets the handler limit for the HTTP/2 server.


298-306: LGTM!

The function correctly configures the use of prohibited cipher suites for the HTTP/2 server.


308-318: LGTM!

The function correctly sets the maximum upload buffer size per connection for the HTTP/2 server.


320-330: LGTM!

The function correctly sets the maximum upload buffer size per stream for the HTTP/2 server.


332-342: LGTM!

The function correctly sets the maximum number of concurrent streams for the HTTP/2 server.


344-354: LGTM!

The function correctly sets the maximum size of the decoder header table for the HTTP/2 server.


356-366: LGTM!

The function correctly sets the maximum size of the encoder header table for the HTTP/2 server.


368-378: LGTM!

The function correctly sets the maximum frame size that can be read by the HTTP/2 server.

k8s/index/job/correction/configmap.yaml (3)

85-94: LGTM!

The HTTP/2 configuration block for port 3000 is correctly set up as a placeholder for future support.


120-129: LGTM!

The HTTP/2 configuration block for port 3001 is correctly set up as a placeholder for future support.


156-165: LGTM!

The HTTP/2 configuration block for port 6060 is correctly set up as a placeholder for future support.

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 6, 2024
@kpango kpango force-pushed the feature/internal/enable-http2-features branch from eeadafc to dfb5df6 Compare August 6, 2024 22:29
Copy link

cloudflare-workers-and-pages bot commented Aug 6, 2024

Deploying vald with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1c30a54
Status: ✅  Deploy successful!
Preview URL: https://124a51ce.vald.pages.dev
Branch Preview URL: https://feature-internal-enable-http.vald.pages.dev

View logs

@kpango kpango force-pushed the feature/internal/enable-http2-features branch from dfb5df6 to 0ca9ba6 Compare August 6, 2024 22:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (3)
k8s/gateway/gateway/lb/configmap.yaml (3)

85-94: Addition of HTTP/2 configuration for port 3000.

The configuration is designed to control HTTP/2 behavior, but the enabled flag is set to false.

Consider enabling HTTP/2 for testing.

-  enabled: false
+  enabled: true

120-129: Addition of HTTP/2 configuration for port 3001.

The configuration is designed to control HTTP/2 behavior, but the enabled flag is set to false.

Consider enabling HTTP/2 for testing.

-  enabled: false
+  enabled: true

156-165: Addition of HTTP/2 configuration for port 6060.

The configuration is designed to control HTTP/2 behavior, but the enabled flag is set to false.

Consider enabling HTTP/2 for testing.

-  enabled: false
+  enabled: true
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 74c85dc and eeadafc.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (34)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • go.mod (4 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/VALD_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (10)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/agent/core/agent/Dockerfile
  • dockers/ci/base/Dockerfile
  • example/client/go.mod
  • k8s/gateway/gateway/lb/deployment.yaml
  • versions/GO_VERSION
  • versions/VALD_VERSION
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional context used
GitHub Check: codecov/patch
internal/net/http/client/client.go

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

Additional comments not posted (124)
dockers/buildkit/syft/scanner/Dockerfile (1)

1-15: Licensing information is correctly included.

The file includes the standard Apache 2.0 license information, which is appropriate and necessary for open-source projects.

.github/workflows/dockers-buildkit-syft-scanner-image.yaml (3)

1-15: Licensing information is correctly included.

The file includes the standard Apache 2.0 license information, which is appropriate and necessary for open-source projects.


16-29: Workflow name and triggers are well-defined.

The workflow name is descriptive and the triggers cover schedule, push, and pull request events, ensuring the image is built regularly and on relevant changes.


42-48: Job definitions are clear and use a predefined workflow.

The job uses a predefined workflow file and specifies the target and platforms, ensuring consistency and compatibility across different architectures.

internal/net/http/client/client.go (4)

Line range hint 1-15:
Licensing information is correctly included.

The file includes the standard Apache 2.0 license information, which is appropriate and necessary for open-source projects.

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests


Line range hint 17-26:
Imports are relevant and necessary.

The imports include packages for HTTP/2 support, backoff, logging, and error handling, which are essential for the functionality provided by the New function.

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests


Line range hint 66-70:
Ensure test coverage for the new transport configuration.

The new transport configuration logic should be thoroughly tested to ensure reliability.

Verification successful

Test coverage for the new transport configuration is verified.

The test cases in internal/net/http/client/client_test.go cover the initialization of the http.Client with the new transport configuration logic, including different scenarios.

  • internal/net/http/client/client_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for the new transport configuration logic.

# Test: Search for tests covering the `New` function. Expect: Tests covering both success and failure scenarios.
rg --type go -A 10 'func TestNew' | rg 'htr.WithRoundTripper'

Length of output: 61


Script:

#!/bin/bash
# Description: Verify test coverage for the new transport configuration logic.

# Search for usage of `htr.WithRoundTripper` in Go test files to identify relevant test cases.
rg --type go 'htr.WithRoundTripper' --glob '*_test.go'

Length of output: 238


Script:

#!/bin/bash
# Description: Extract and review the relevant portions of `client_test.go` to verify test coverage for the new transport configuration logic.

# Extract lines around the usage of `htr.WithRoundTripper` in `client_test.go`.
rg --type go -A 10 -B 10 'htr.WithRoundTripper' internal/net/http/client/client_test.go

Length of output: 995

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests


52-61: Enhanced error handling for HTTP/2 transport configuration.

The updated implementation improves error handling by returning a client with exponential backoff if HTTP/2 configuration fails. However, ensure this new logic is covered by tests.

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

.github/workflows/helm.yml (1)

38-38: LGTM! Verify the necessity of the added host option.

The addition of --add-host host.docker.internal:host-gateway enhances the container's networking capabilities by allowing it to resolve host.docker.internal to the host machine's gateway. Ensure that this option is necessary for the intended functionality.

k8s/index/operator/deployment.yaml (1)

49-49: LGTM! Verify the integrity of the new configuration.

The checksum for the configmap has been updated, indicating that the configuration data has changed. Ensure that the new configuration is correct and that the deployment reflects the latest configuration state.

k8s/discoverer/deployment.yaml (1)

49-49: LGTM! Verify the integrity of the new configuration.

The checksum for the configmap has been updated, indicating that the configuration data has changed. Ensure that the new configuration is correct and that the deployment reflects the latest configuration state.

k8s/manager/index/deployment.yaml (1)

49-49: Confirm the purpose of the checksum update.

The change in the checksum value indicates that the associated ConfigMap has been updated. Ensure that the ConfigMap has been updated accordingly and that this change is intended to trigger a rolling update of the Deployment.

k8s/agent/ngt/configmap.yaml (3)

85-94: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the liveness probe on port 3000 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.


120-129: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the readiness probe on port 3001 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.


156-165: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the metrics server on port 6060 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.

k8s/discoverer/configmap.yaml (3)

85-94: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the liveness probe on port 3000 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.


120-129: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the readiness probe on port 3001 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.


156-165: Confirm the purpose of adding HTTP/2 settings.

The HTTP/2 settings for the metrics server on port 6060 are currently disabled (enabled: false). Ensure that the default values are appropriate for future use and confirm the purpose of adding these settings.

internal/servers/server/server.go (5)

43-43: Import statement for HTTP/2 support.

The addition of the http2 package is necessary for the new HTTP/2 functionality.


94-95: New fields for HTTP/2 support.

The fields h2srv and enableH2 are necessary to manage HTTP/2 server settings and enable/disable HTTP/2 support.


184-184: Retention of SetKeepAlivesEnabled method call.

The SetKeepAlivesEnabled method call ensures that keep-alive connections are still managed effectively.


184-204: Initialization of the starter function in the New function.

The New function correctly initializes the starter function to handle HTTP/2 settings.


190-204: Modification of the starter function for HTTP/2 support.

The changes ensure that HTTP/2 settings are applied when enabled.

Verify the correct configuration of HTTP/2 settings.

Verification successful

Modification of the starter function for HTTP/2 support.

The changes ensure that HTTP/2 settings are applied when enabled. The initialization and configuration of srv.http.enableH2 and srv.http.h2srv are correctly handled in internal/servers/server/option.go.

  • srv.http.enableH2 is set in internal/servers/server/option.go.
  • srv.http.h2srv is initialized multiple times in internal/servers/server/option.go.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correct configuration of HTTP/2 settings.

# Test: Search for HTTP/2 configuration in the server setup. Expect: Correct configuration of HTTP/2 settings.
rg --type go -A 10 'http2.ConfigureServer'

Length of output: 635


Script:

#!/bin/bash
# Description: Verify the initialization and configuration of srv.http.enableH2 and srv.http.h2srv.

# Test: Search for the initialization and configuration of srv.http.enableH2.
rg --type go 'srv.http.enableH2'

# Test: Search for the initialization and configuration of srv.http.h2srv.
rg --type go 'srv.http.h2srv'

Length of output: 362


Script:

#!/bin/bash
# Description: Verify the initialization and configuration of srv.http.enableH2 and srv.http.h2srv.

# Test: Search for the initialization and configuration of srv.http.enableH2.
rg --type go 'enableH2\s*=\s*'

# Test: Search for the initialization and configuration of srv.http.h2srv.
rg --type go 'h2srv\s*=\s*'

Length of output: 1186

Makefile.d/docker.mk (3)

28-28: Addition of new build target for buildkit-syft-scanner.

The new target is necessary for building the buildkit-syft-scanner image.


71-71: Modification of build arguments for SBOM generation.

The new --attest argument specifies the type of attestations and the generator used, improving the handling of SBOM generation.


223-233: Detailed configuration for buildkit-syft-scanner.

The configuration ensures that the buildkit-syft-scanner image is built correctly using the specified Dockerfile.

k8s/index/job/save/configmap.yaml (3)

120-129: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

However, ensure that the new settings are correctly utilized and tested.


156-165: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

However, ensure that the new settings are correctly utilized and tested.


85-94: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

However, ensure that the new settings are correctly utilized and tested.

Verification successful

LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

However, ensure that the new settings are correctly utilized and tested.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new HTTP/2 settings in the codebase.

# Test: Search for the usage of the HTTP/2 settings. Expect: Only occurrences of the new settings.
rg --type yaml -A 5 $'http2:'

Length of output: 102101

k8s/index/job/creation/configmap.yaml (3)

85-94: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

However, ensure that the new settings are correctly utilized and tested.


120-129: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.


156-165: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

k8s/manager/index/configmap.yaml (3)

85-94: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.


120-129: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.


156-165: LGTM! Verify the usage of the new HTTP/2 settings.

The HTTP/2 settings are added with default values and are disabled by default, ensuring backward compatibility.

internal/config/server.go (5)

32-36: Enhance TLS and shutdown configuration.

The addition of TLS and FullShutdownDuration fields improves the configurability and control over server shutdown behavior and security configurations.


56-66: Reposition the Port field for clarity.

Repositioning the Port field improves the readability and organization of the Server struct.


71-71: Add HTTP/2 configuration.

The addition of the HTTP2 field allows for HTTP/2 specific configurations, enhancing the flexibility of the HTTP server settings.


81-91: Add detailed HTTP/2 configurations.

The HTTP2 struct includes fields that provide granular control over various aspects of HTTP/2 connections, such as handler limits, buffer sizes, and concurrent streams.


95-96: Add gRPC connection timeout configuration.

The addition of the ConnectionTimeout field enhances the configurability of gRPC server settings, allowing for better control over connection timeouts.

internal/servers/server/option.go (9)

279-284: Enable or disable HTTP/2 support.

The WithHTTP2Enabled function correctly implements the enabling or disabling of HTTP/2 support.


286-296: Set maximum number of handlers for HTTP/2.

The WithHandlerLimit function sets the maximum number of handlers for HTTP/2 and includes logic to initialize the http2.Server if it is not already initialized.


298-306: Configure prohibited cipher suites.

The WithPermitProhibitedCipherSuites function configures whether to permit prohibited cipher suites and includes logic to initialize the http2.Server if it is not already initialized.


308-318: Specify maximum upload buffer size per connection.

The WithMaxUploadBufferPerConnection function specifies the maximum upload buffer size per connection and includes logic to initialize the http2.Server if it is not already initialized.


320-330: Set maximum upload buffer size per stream.

The WithMaxUploadBufferPerStream function sets the maximum upload buffer size per stream and includes logic to initialize the http2.Server if it is not already initialized.


332-342: Configure maximum number of concurrent streams.

The WithMaxConcurrentStreams function configures the maximum number of concurrent streams and includes logic to initialize the http2.Server if it is not already initialized.


344-354: Set maximum size of decoder header table.

The WithMaxDecoderHeaderTableSize function sets the maximum size of the decoder header table and includes logic to initialize the http2.Server if it is not already initialized.


356-366: Specify maximum size of encoder header table.

The WithMaxEncoderHeaderTableSize function specifies the maximum size of the encoder header table and includes logic to initialize the http2.Server if it is not already initialized.


368-378: Set maximum size of read frames.

The WithMaxReadFrameSize function sets the maximum size of read frames and includes logic to initialize the http2.Server if it is not already initialized.

k8s/index/job/correction/configmap.yaml (3)

85-94: Add HTTP/2 configuration block for port 3000.

The HTTP/2 configuration block includes several parameters that provide default values for various aspects of HTTP/2 connections, even though HTTP/2 is disabled.


120-129: Add HTTP/2 configuration block for port 3001.

The HTTP/2 configuration block includes several parameters that provide default values for various aspects of HTTP/2 connections, even though HTTP/2 is disabled.


156-165: Add HTTP/2 configuration block for port 6060.

The HTTP/2 configuration block includes several parameters that provide default values for various aspects of HTTP/2 connections, even though HTTP/2 is disabled.

k8s/index/operator/configmap.yaml (2)

28-28: Verify correctness of HTTP/2 settings.

The new HTTP/2 settings introduce parameters such as enabled, handler_limit, max_concurrent_streams, and various size limits for headers and frames. Ensure these settings are correctly configured and tested.


28-28: Verify correctness of TCP socket options.

The refined TCP socket options include settings such as tcp_fast_open and tcp_no_delay. Ensure these options are correctly configured and tested for performance improvements.

Makefile (2)

35-35: Verify correctness of the new variable definition.

The new variable BUILDKIT_SYFT_SCANNER_IMAGE is defined to hold the value $(NAME)-buildkit-syft-scanner. Ensure this variable is correctly defined and used in the Makefile.


482-488: Verify correctness of the new target definition.

The new target remove/empty/file is a phony target that executes a shell command to find and remove empty files. Ensure this target is correctly defined and used in the Makefile.

go.mod (8)

18-18: Verify compatibility of the updated version.

Ensure that the new version v0.0.0-20240806182212-6cf545ebdd6b of code.cloudfoundry.org/bytefmt is compatible with the existing codebase.


297-297: Verify compatibility of the updated version.

Ensure that the new version v0.26.0 of golang.org/x/crypto is compatible with the existing codebase.

Also applies to: 508-508


300-300: Verify compatibility of the updated version.

Ensure that the new version v0.19.0 of golang.org/x/image is compatible with the existing codebase.

Also applies to: 512-512


304-304: Verify compatibility of the updated version.

Ensure that the new version v0.28.0 of golang.org/x/net is compatible with the existing codebase.

Also applies to: 399-399


306-306: Verify compatibility of the updated version.

Ensure that the new version v0.8.0 of golang.org/x/sync is compatible with the existing codebase.

Also applies to: 401-401


309-309: Verify compatibility of the updated version.

Ensure that the new version v0.17.0 of golang.org/x/text is compatible with the existing codebase.

Also applies to: 403-403


308-308: Verify compatibility of the updated version.

Ensure that the new version v0.23.0 of golang.org/x/term is compatible with the existing codebase.

Also applies to: 513-513


311-311: Verify compatibility of the updated version.

Ensure that the new version v0.24.0 of golang.org/x/tools is compatible with the existing codebase.

Also applies to: 514-514

charts/vald/values.yaml (8)

97-125: LGTM!

The HTTP/2 configuration parameters are well-defined and cover essential aspects of HTTP/2 configuration.


388-407: LGTM!

The HTTP/2 configuration parameters are well-defined and cover essential aspects of HTTP/2 configuration.


508-526: LGTM!

The HTTP/2 configuration parameters are well-defined and cover essential aspects of HTTP/2 configuration.


601-619: LGTM!

The HTTP/2 configuration parameters are well-defined and cover essential aspects of HTTP/2 configuration.


97-125: Verify the necessity of permitting prohibited cipher suites.

The permit_prohibited_cipher_suites parameter is set to true, which might introduce security risks. Ensure that this setting is necessary and does not compromise security.


508-526: Verify the necessity of permitting prohibited cipher suites.

The permit_prohibited_cipher_suites parameter is set to true, which might introduce security risks. Ensure that this setting is necessary and does not compromise security.


388-407: Verify the necessity of permitting prohibited cipher suites.

The permit_prohibited_cipher_suites parameter is set to true, which might introduce security risks. Ensure that this setting is necessary and does not compromise security.


601-619: Verify the necessity of permitting prohibited cipher suites.

The permit_prohibited_cipher_suites parameter is set to true, which might introduce security risks. Ensure that this setting is necessary and does not compromise security.

charts/vald-helm-operator/crds/valdrelease.yaml (23)

702-722: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunk.

However, ensure that these properties are correctly utilized in the codebase.


831-851: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


1041-1061: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


1546-1566: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


1663-1683: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


1792-1812: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


2002-2022: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


2481-2501: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


2598-2618: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


2727-2747: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


2937-2957: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


3432-3452: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


3549-3569: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


3678-3698: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


3888-3908: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


4887-4907: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


5004-5024: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


5133-5153: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


5343-5363: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


6315-6335: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


6432-6452: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


6561-6581: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is consistent with the previous hunks.

However, ensure that these properties are correctly utilized in the codebase.


585-605: LGTM! Verify the usage of these properties in the codebase.

The addition of the http2 property and its sub-properties is well-structured and follows the standard YAML schema conventions.

However, ensure that these properties are correctly utilized in the codebase.

Verification successful

The http2 properties are correctly utilized in the codebase.

The http2 properties added in charts/vald-helm-operator/crds/valdrelease.yaml are being utilized in the application logic files, particularly in the server configuration and HTTP client files.

  • internal/servers/server/server.go
  • internal/servers/server/option.go
  • internal/net/http/client/client_test.go
  • internal/net/http/client/client.go

These findings confirm that the properties are integrated and used as intended.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the newly added HTTP/2 properties in the codebase.

# Test: Search for the usage of the `http2` properties. Expect: Occurrences of the new properties.
rg --type yaml -A 5 $'http2:'

Length of output: 102101


Script:

#!/bin/bash
# Description: Verify the usage of the newly added HTTP/2 properties in the application logic.

# Test: Search for the usage of the `http2` properties in the codebase. Expect: Occurrences of the new properties in application logic files.
rg 'http2\.' --type go --type js --type ts --type py

Length of output: 1022

k8s/operator/helm/crds/valdrelease.yaml (29)

585-605: Ensure proper documentation for new HTTP/2 properties.

The new http2 properties are well-defined, but ensure that they are properly documented in the CRD documentation to guide users on how to configure these settings.


702-722: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunk. Ensure that all instances of these properties are uniformly defined across the CRD.


831-851: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


1041-1061: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


1546-1566: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


1663-1683: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


1792-1812: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


2002-2022: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


2481-2501: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


2598-2618: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


2727-2747: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


2937-2957: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


3432-3452: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


3549-3569: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


3678-3698: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


3888-3908: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


4887-4907: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


5004-5024: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


5133-5153: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


5343-5363: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


6315-6335: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


6432-6452: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


6561-6581: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


6771-6791: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


7463-7483: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


7580-7600: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


7709-7729: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


7919-7939: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the previous hunks. Ensure that all instances of these properties are uniformly defined across the CRD.


8795-8815: Consistency check for HTTP/2 properties.

The http2 properties are consistent with the

Comments failed to post (1)
dockers/buildkit/syft/scanner/Dockerfile

18-18: Specify a stable version for the base image.

Using the edge tag for the base image can lead to unpredictable builds due to potential changes in the image. It's better to specify a stable version.

- FROM docker/buildkit-syft-scanner:edge AS scanner
+ FROM docker/buildkit-syft-scanner:stable AS scanner
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.

FROM docker/buildkit-syft-scanner:stable AS scanner

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 74c85dc and eeadafc.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (34)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • go.mod (4 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/VALD_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (8)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/agent/core/agent/Dockerfile
  • dockers/ci/base/Dockerfile
  • versions/GO_VERSION
  • versions/VALD_VERSION
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional context used
GitHub Check: codecov/patch
internal/net/http/client/client.go

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

Additional comments not posted (130)
dockers/buildkit/syft/scanner/Dockerfile (2)

1-15: Metadata looks good.

The licensing and metadata information is correctly included.


18-18: Verify the base image.

Ensure that the base image docker/buildkit-syft-scanner:edge is appropriate for your use case and is secure.

.github/workflows/dockers-buildkit-syft-scanner-image.yaml (3)

1-15: Metadata looks good.

The licensing and metadata information is correctly included.


42-48: Verify the job definition.

Ensure that the job definition for building the Docker image is correct and uses the appropriate workflow and platforms.

Verification successful

The job definition is correct.

The job definition for building the Docker image in the .github/workflows/_docker-image.yaml file is correct and uses the appropriate workflow and platforms.

  • The build job uses the target and platforms inputs correctly.
  • The platforms specified (linux/amd64 and linux/arm64) are supported.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the job definition in the GitHub Actions workflow.

# Test: Check if the referenced workflow file exists and is valid.
if [ ! -f ".github/workflows/_docker-image.yaml" ]; then
  echo "Referenced workflow file not found: .github/workflows/_docker-image.yaml"
fi

# Test: Check if the specified platforms are supported.
echo "Supported platforms: linux/amd64, linux/arm64"

Length of output: 154


Script:

#!/bin/bash
# Description: Display the contents of the referenced workflow file to verify the job definition.

# Display the contents of the workflow file
cat .github/workflows/_docker-image.yaml

Length of output: 4446


17-29: Verify the triggers and paths.

Ensure that the triggers and paths specified for the workflow are correct and align with your CI/CD practices.

Verification successful

The triggers and paths in the workflow file are correctly specified.

The detailed branch names and tag patterns in .github/workflows/dockers-buildkit-syft-scanner-image.yaml align with the CI/CD practices as specified in the review comment.

  • Branches:
    • main
    • release/v*.*
    • !release/v*.*.*
  • Tags:
    • *.*.*
    • v*.*.*
    • *.*.*-*
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the triggers and paths in the GitHub Actions workflow.

# Test: Check if the specified paths exist in the repository.
for path in ".github/actions/docker-build/action.yaml" ".github/workflows/_docker-image.yaml" ".github/workflows/dockers-buildkit-syft-scanner-image.yml" "dockers/buildkit/syft/scanner/Dockerfile"; do
  if [ ! -f "$path" ]; then
    echo "Path not found: $path"
  fi
done

Length of output: 1197


Script:

#!/bin/bash
# Description: Verify the existence of the correct workflow file and check its triggers and paths.

# Check if the correct workflow file exists.
if [ -f ".github/workflows/dockers-buildkit-syft-scanner-image.yaml" ]; then
  echo "File exists: .github/workflows/dockers-buildkit-syft-scanner-image.yaml"
  
  # Display the triggers and paths specified in the workflow file.
  echo "Triggers and paths in .github/workflows/dockers-buildkit-syft-scanner-image.yaml:"
  grep -E 'on:|schedule:|cron:|push:|branches:|tags:' .github/workflows/dockers-buildkit-syft-scanner-image.yaml
else
  echo "File not found: .github/workflows/dockers-buildkit-syft-scanner-image.yaml"
fi

Length of output: 582


Script:

#!/bin/bash
# Description: Verify the detailed branch names and tag patterns in the workflow file.

# Display the detailed branch names and tag patterns in the workflow file.
echo "Detailed branches and tags in .github/workflows/dockers-buildkit-syft-scanner-image.yaml:"
grep -A 3 'branches:' .github/workflows/dockers-buildkit-syft-scanner-image.yaml
grep -A 3 'tags:' .github/workflows/dockers-buildkit-syft-scanner-image.yaml

Length of output: 485

internal/net/http/client/client.go (1)

Line range hint 66-70:
Improved error handling looks good.

The use of the updated transport settings with exponential backoff for retries enhances the resilience of the HTTP client.

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

example/client/go.mod (6)

11-11: Dependency Update: golang.org/x/crypto

The version for golang.org/x/crypto has been updated from v0.25.0 to v0.26.0. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.


12-12: Dependency Update: golang.org/x/net

The version for golang.org/x/net has been updated from v0.27.0 to v0.28.0. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.


13-13: Dependency Update: golang.org/x/text

The version for golang.org/x/text has been updated from v0.16.0 to v0.17.0. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.


26-26: Dependency Update: github.com/vdaas/vald-client-go

The version for github.com/vdaas/vald-client-go has been updated from v1.7.12 to v1.7.13. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.


32-32: Dependency Update: buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go

The version for buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go has been updated from v1.32.0-20240221180331-f05a6f4403ce.1 to v1.34.2-20240717164558-a6c49f84cc0f.2. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.


37-37: Dependency Update: golang.org/x/sys

The version for golang.org/x/sys has been updated from v0.22.0 to v0.23.0. Ensure that this update does not introduce any breaking changes and is compatible with the rest of the codebase.

.github/workflows/helm.yml (1)

38-38: Addition of Container Option: --add-host host.docker.internal:host-gateway

The addition of the --add-host host.docker.internal:host-gateway option enhances the container's networking capabilities by enabling it to communicate with services running on the host. This is particularly useful for integration tests or accessing resources not exposed through the container network.

k8s/index/operator/deployment.yaml (1)

49-49: Update of checksum/configmap Annotation

The checksum/configmap annotation has been updated to eae6b3eac702f445f9b5a0d1495af9917479303b103f986c10b5b3db9d749086. This ensures that the deployment recognizes and utilizes the latest configuration data from the updated ConfigMap.

k8s/discoverer/deployment.yaml (1)

49-49: Checksum update for ConfigMap detected.

The checksum/configmap annotation has been updated to ensure the deployment references the latest configuration. Ensure that the new checksum is accurate and that the ConfigMap changes are correctly reflected.

k8s/manager/index/deployment.yaml (1)

49-49: Checksum update for ConfigMap detected.

The checksum/configmap annotation has been updated to ensure the deployment references the latest configuration. Ensure that the new checksum is accurate and that the ConfigMap changes are correctly reflected.

k8s/gateway/gateway/lb/deployment.yaml (1)

48-48: Checksum update for ConfigMap detected.

The checksum/configmap annotation has been updated to ensure the deployment references the latest configuration. Ensure that the new checksum is accurate and that the ConfigMap changes are correctly reflected.

k8s/agent/ngt/configmap.yaml (3)

85-94: HTTP/2 configuration block for port 3000 looks good.

The parameters are correctly defined and consistent with the overall configuration.


120-129: HTTP/2 configuration block for port 3001 looks good.

The parameters are correctly defined and consistent with the overall configuration.


156-165: HTTP/2 configuration block for port 6060 looks good.

The parameters are correctly defined and consistent with the overall configuration.

k8s/discoverer/configmap.yaml (3)

85-94: HTTP/2 configuration block for port 3000 looks good.

The parameters are correctly defined and consistent with the overall configuration.


120-129: HTTP/2 configuration block for port 3001 looks good.

The parameters are correctly defined and consistent with the overall configuration.


156-165: HTTP/2 configuration block for port 6060 looks good.

The parameters are correctly defined and consistent with the overall configuration.

internal/servers/server/server.go (3)

43-43: Import of http2 package looks good.

The import is correctly added and necessary for HTTP/2 support.


94-95: New fields h2srv and enableH2 in server struct look good.

The fields are correctly added and necessary for HTTP/2 support.


190-204: HTTP/2 configuration logic in New function looks good.

The logic is correctly added and necessary for HTTP/2 support.

Makefile.d/docker.mk (3)

28-28: LGTM! Addition of docker/build/buildkit-syft-scanner target is appropriate.

This ensures that the buildkit-syft-scanner image is built as part of the overall Docker image construction process.


71-71: LGTM! Update to use --attest for SBOM generation.

This change enhances security measures by specifying the type of attestation and the generator being used.


223-233: LGTM! Addition of docker/name/buildkit-syft-scanner and docker/build/buildkit-syft-scanner targets is appropriate.

These targets facilitate the integration of the buildkit-syft-scanner into the build pipeline by defining the Dockerfile location and the image name.

k8s/gateway/gateway/lb/configmap.yaml (3)

85-94: LGTM! Addition of HTTP/2 configuration for port 3000 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.


120-129: LGTM! Addition of HTTP/2 configuration for port 3001 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.


156-165: LGTM! Addition of HTTP/2 configuration for port 6060 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.

k8s/index/job/save/configmap.yaml (3)

85-94: LGTM! Addition of HTTP/2 configuration for port 3000 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.


120-129: LGTM! Addition of HTTP/2 configuration for port 3001 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.


156-165: LGTM! Addition of HTTP/2 configuration for port 6060 is appropriate.

The new http2 block provides granular control over HTTP/2 behavior with various parameters.

k8s/index/job/creation/configmap.yaml (2)

120-129: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.


156-165: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.

k8s/manager/index/configmap.yaml (3)

85-94: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.


120-129: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.


156-165: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.

internal/config/server.go (6)

32-36: LGTM! But verify the usage of new fields.

The additions of TLS and FullShutdownDuration fields enhance the configuration capabilities.

However, ensure that these fields are correctly utilized in the codebase.


56-66: LGTM! But verify the impact of field relocation.

The reordering of fields in the Server struct improves clarity and organization.

However, ensure that the relocation of the Port field does not affect the existing logic.

Verification successful

The relocation of the Port field within the Server struct does not impact the existing logic.

The search results confirm that the Port field is accessed by its name across the codebase, ensuring that its position within the struct does not affect the functionality.

  • No changes are required based on the reordering of the Port field in the Server struct.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of the relocation of the `Port` field.

# Test: Search for the usage of the `Port` field. Expect: No impact on existing logic.
rg --type go 'Port'

Length of output: 68914


71-71: LGTM! But verify the usage of the new field.

The addition of the HTTP2 field enhances the configuration capabilities.

However, ensure that this field is correctly utilized in the codebase.


95-99: LGTM! But verify the usage of new fields.

The additions of Keepalive, ConnectionTimeout, Interceptors, EnableReflection, and EnableAdmin fields enhance the configuration capabilities for gRPC.

However, ensure that these fields are correctly utilized in the codebase.

Verification successful

LGTM! But verify the usage of new fields.

The additions of Keepalive, ConnectionTimeout, Interceptors, EnableReflection, and EnableAdmin fields enhance the configuration capabilities for gRPC.

The search results indicate that these fields are being utilized in the codebase, including in configuration files, server options, and tests. Ensure that these usages are correct and consistent with the intended functionality.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of new fields in the `GRPC` struct.

# Test: Search for the usage of the new fields. Expect: Correct usage in the codebase.
rg --type go 'Keepalive|ConnectionTimeout|Interceptors|EnableReflection|EnableAdmin'

Length of output: 30143


274-286: LGTM! But verify the implementation of conditional logic.

The changes to the Opts method ensure a dynamic approach to server configuration based on the Enabled flag for HTTP/2.

However, ensure that the conditional logic is correctly implemented.

Verification successful

The conditional logic for HTTP/2 options is correctly implemented.

The opts slice is appropriately appended with HTTP/2 settings only when s.HTTP.HTTP2.Enabled is true, ensuring a dynamic server configuration based on the HTTP/2 enabled flag.

  • internal/config/server.go: Lines 274-286
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of conditional logic for HTTP/2 options.

# Test: Search for the implementation of the conditional logic. Expect: Correct implementation in the codebase.
rg --type go 'if s.HTTP.HTTP2.Enabled'

Length of output: 95


Script:

#!/bin/bash
# Description: Extract the relevant lines of code around the conditional logic for HTTP/2 options.

# Extract 10 lines before and after the match to review the surrounding code.
rg --type go 'if s.HTTP.HTTP2.Enabled' -A 10 -B 10

Length of output: 1672


80-91: LGTM! But verify the usage of the new struct.

The HTTP2 struct provides comprehensive configuration options for HTTP/2.

However, ensure that these options are correctly utilized in the codebase.

Verification successful

Verification Successful!

The HTTP2 struct and its fields are correctly utilized in the codebase. The fields are referenced and used appropriately in various parts of the configuration setup.

  • HandlerLimit is used in internal/config/server.go and internal/servers/server/option.go.
  • Enabled is used in internal/config/server.go and other related files.
  • PermitProhibitedCipherSuites is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxUploadBufferPerConnection is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxUploadBufferPerStream is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxConcurrentStreams is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxDecoderHeaderTableSize is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxEncoderHeaderTableSize is used in internal/config/server.go and internal/servers/server/option.go.
  • MaxReadFrameSize is used in internal/config/server.go and internal/servers/server/option.go.

No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new `HTTP2` struct.

# Test: Search for the usage of the new struct. Expect: Correct usage in the codebase.
rg --type go 'HTTP2'

Length of output: 2763


Script:

#!/bin/bash
# Description: Verify the usage of each field within the `HTTP2` struct.

# Test: Search for the usage of each field in the `HTTP2` struct. Expect: Correct usage in the codebase.
fields=(
  "HandlerLimit"
  "Enabled"
  "PermitProhibitedCipherSuites"
  "MaxUploadBufferPerConnection"
  "MaxUploadBufferPerStream"
  "MaxConcurrentStreams"
  "MaxDecoderHeaderTableSize"
  "MaxEncoderHeaderTableSize"
  "MaxReadFrameSize"
)

for field in "${fields[@]}"; do
  echo "Searching for field: $field"
  rg --type go "$field"
done

Length of output: 54248

internal/servers/server/option.go (9)

279-284: LGTM!

The function WithHTTP2Enabled correctly modifies the enableH2 field of the http struct to enable or disable HTTP/2 support.


286-296: LGTM!

The function WithHandlerLimit correctly sets the MaxHandlers field of the h2srv struct after ensuring it is initialized.


298-306: LGTM!

The function WithPermitProhibitedCipherSuites correctly sets the PermitProhibitedCipherSuites field of the h2srv struct after ensuring it is initialized.


308-318: LGTM!

The function WithMaxUploadBufferPerConnection correctly sets the MaxUploadBufferPerConnection field of the h2srv struct after ensuring it is initialized.


320-330: LGTM!

The function WithMaxUploadBufferPerStream correctly sets the MaxUploadBufferPerStream field of the h2srv struct after ensuring it is initialized.


332-342: LGTM!

The function WithMaxConcurrentStreams correctly sets the MaxConcurrentStreams field of the h2srv struct after ensuring it is initialized.


344-354: LGTM!

The function WithMaxDecoderHeaderTableSize correctly sets the MaxDecoderHeaderTableSize field of the h2srv struct after ensuring it is initialized.


356-366: LGTM!

The function WithMaxEncoderHeaderTableSize correctly sets the MaxEncoderHeaderTableSize field of the h2srv struct after ensuring it is initialized.


368-378: LGTM!

The function WithMaxReadFrameSize correctly sets the MaxReadFrameSize field of the h2srv struct after ensuring it is initialized.

k8s/index/job/correction/configmap.yaml (3)

85-94: LGTM!

The HTTP/2 configuration block for the liveness server on port 3000 is comprehensive and includes all necessary settings for HTTP/2 support.


120-129: LGTM!

The HTTP/2 configuration block for the readiness server on port 3001 is comprehensive and includes all necessary settings for HTTP/2 support.


156-165: LGTM!

The HTTP/2 configuration block for the metrics server on port 6060 is comprehensive and includes all necessary settings for HTTP/2 support.

k8s/index/operator/configmap.yaml (3)

Line range hint 28-94: LGTM!

The HTTP/2 configuration block for the liveness server is comprehensive and includes all necessary settings for HTTP/2 support.


Line range hint 95-129: LGTM!

The HTTP/2 configuration block for the readiness server is comprehensive and includes all necessary settings for HTTP/2 support.


Line range hint 130-165: LGTM!

The HTTP/2 configuration block for the metrics server is comprehensive and includes all necessary settings for HTTP/2 support.

Makefile (2)

35-35: LGTM!

The addition of BUILDKIT_SYFT_SCANNER_IMAGE is straightforward and does not introduce any issues.


482-488: LGTM!

The addition of the remove/empty/file target enhances the formatting process by removing unnecessary empty files.

go.mod (7)

18-18: LGTM!

The update of code.cloudfoundry.org/bytefmt to a newer version is a routine version bump.


297-297: LGTM! But verify for breaking changes.

The update of golang.org/x/crypto to a newer version is likely to include security and performance improvements. Ensure that the updated version does not introduce breaking changes.

Verification successful

No issues found with the update of golang.org/x/crypto.

The dependency golang.org/x/crypto is listed as an indirect dependency in the go.mod file, indicating it is not directly used in the codebase. Therefore, the update should not introduce any breaking changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/crypto`.

# Test: Search for the usage of `golang.org/x/crypto`. Expect: No breaking changes.
rg --type go 'golang.org/x/crypto'

Length of output: 34


Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/crypto`.

# Test: Search for specific sub-packages or modules within `golang.org/x/crypto`. Expect: No breaking changes.
rg --type go 'golang.org/x/crypto/ssh' -A 5
rg --type go 'golang.org/x/crypto/bcrypt' -A 5
rg --type go 'golang.org/x/crypto/acme' -A 5
rg --type go 'golang.org/x/crypto/argon2' -A 5

Length of output: 182


Script:

#!/bin/bash
# Description: Verify the presence of the `go.mod` file and check if `golang.org/x/crypto` is listed as a dependency.

# Check if the `go.mod` file exists.
if [ -f "go.mod" ]; then
  # Search for `golang.org/x/crypto` in the `go.mod` file.
  rg 'golang.org/x/crypto' go.mod
else
  echo "go.mod file not found."
fi

Length of output: 144


304-304: LGTM! But verify for breaking changes.

The update of golang.org/x/net to a newer version is likely to include security and performance improvements. Ensure that the updated version does not introduce breaking changes.


300-300: LGTM! But verify for breaking changes.

The update of golang.org/x/image to a newer version is likely to include new features and bug fixes. Ensure that the updated version does not introduce breaking changes.

Verification successful

No issues found with the update.

The golang.org/x/image package is not actively used in the codebase, so updating it should not introduce any breaking changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/image`.

# Test: Search for the usage of `golang.org/x/image`. Expect: No breaking changes.
rg --type go 'golang.org/x/image'

Length of output: 33


Script:

#!/bin/bash
# Description: Search for import statements of `golang.org/x/image` in Go files.

# Test: Search for import statements that include `golang.org/x/image`.
rg --type go 'import\s+"golang.org/x/image' -A 2

Length of output: 50


Script:

#!/bin/bash
# Description: Search for any usage of `golang.org/x/image` under different aliases or indirect usages in Go files.

# Test: Search for any usage of `golang.org/x/image` under different aliases.
rg --type go 'golang.org/x/image'
rg --type go 'image'

Length of output: 2715


Script:

#!/bin/bash
# Description: Search for references to `golang.org/x/image` in build or configuration files.

# Test: Search for references to `golang.org/x/image` in Makefile, Dockerfile, and YAML files.
rg 'golang.org/x/image' -t make -t docker -t yaml

Length of output: 49


308-308: LGTM! But verify for breaking changes.

The update of golang.org/x/term to a newer version is likely to include new features and bug fixes. Ensure that the updated version does not introduce breaking changes.

Verification successful

No issues found with the update of golang.org/x/term.

The dependency golang.org/x/term is not used in the codebase, so updating it should not introduce any breaking changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/term`.

# Test: Search for the usage of `golang.org/x/term`. Expect: No breaking changes.
rg --type go 'golang.org/x/term'

Length of output: 32


Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/term`.

# Test: Search for the usage of `golang.org/x/term` with additional context lines to ensure no breaking changes.
rg --type go 'golang.org/x/term' -A 5 -B 5

Length of output: 42


Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/term`.

# Test: Search for import statements of `golang.org/x/term` to ensure it is used in the codebase.
rg --type go 'import "golang.org/x/term"'

Length of output: 43


Script:

#!/bin/bash
# Description: Verify that there are no breaking changes introduced by the update of `golang.org/x/term`.

# Test: Search for references to `term` within the `golang.org/x` module to ensure it is not used in the codebase.
rg --type go 'golang.org/x/term'

Length of output: 32


309-309: LGTM! But verify for breaking changes.

The update of golang.org/x/text to a newer version is likely to include new features and bug fixes. Ensure that the updated version does not introduce breaking changes.


311-311: LGTM! But verify for breaking changes.

The update of golang.org/x/tools to a newer version is likely to include new features and bug fixes. Ensure that the updated version does not introduce breaking changes.

charts/vald/values.yaml (4)

97-125: Comprehensive HTTP/2 Configuration for REST Server

The HTTP/2 configuration parameters for the REST server are well-defined and cover various aspects of HTTP/2 behavior. Ensure these configurations are documented in the relevant user guides so users can understand and utilize these settings effectively.


388-407: Comprehensive HTTP/2 Configuration for Liveness Server

The HTTP/2 configuration parameters for the liveness server are well-defined and cover various aspects of HTTP/2 behavior. Ensure these configurations are documented in the relevant user guides so users can understand and utilize these settings effectively.


507-526: Comprehensive HTTP/2 Configuration for Readiness Server

The HTTP/2 configuration parameters for the readiness server are well-defined and cover various aspects of HTTP/2 behavior. Ensure these configurations are documented in the relevant user guides so users can understand and utilize these settings effectively.


600-619: Comprehensive HTTP/2 Configuration for Metrics Server

The HTTP/2 configuration parameters for the metrics server are well-defined and cover various aspects of HTTP/2 behavior. Ensure these configurations are documented in the relevant user guides so users can understand and utilize these settings effectively.

charts/vald-helm-operator/crds/valdrelease.yaml (30)

585-605: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


702-722: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


831-851: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


1041-1061: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


1546-1566: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


1663-1683: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


1792-1812: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


2002-2022: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


2481-2501: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


2598-2618: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


2727-2747: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


2937-2957: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


3432-3452: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


3549-3569: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


3678-3698: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


3888-3908: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


4887-4907: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


5004-5024: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


5133-5153: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


5343-5363: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


6315-6335: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


6432-6452: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


6561-6581: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


6771-6791: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


7463-7483: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


7580-7600: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


7709-7729: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


7919-7939: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


8795-8815: HTTP/2 configuration properties added correctly.

The added properties for HTTP/2 configuration are consistent and well-structured.


8912-8932: **HTTP

k8s/operator/helm/crds/valdrelease.yaml (26)

585-605: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


702-722: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


831-851: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


1041-1061: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


1546-1566: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


1663-1683: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


1792-1812: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


2002-2022: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


2481-2501: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


2598-2618: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


2727-2747: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


2937-2957: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


3432-3452: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


3549-3569: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


3678-3698: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


3888-3908: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


4887-4907: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


5004-5024: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


5133-5153: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


5343-5363: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


6315-6335: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


6432-6452: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


6561-6581: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


6771-6791: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


7463-7483: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and align with the expected schema.


7580-7600: HTTP/2 configuration added correctly.

The http2 object and its properties are well-defined and

Comments failed to post (2)
k8s/index/job/creation/configmap.yaml

85-94: Security Risk: Permit Prohibited Cipher Suites

The permit_prohibited_cipher_suites parameter is set to true, which might pose a security risk if HTTP/2 is enabled in the future. Consider setting it to false to avoid potential vulnerabilities.

internal/net/http/client/client.go

52-61: Improved error handling looks good.

The use of http2.ConfigureTransports and the new transport with exponential backoff for retries enhances the resilience of the HTTP client.

However, the added lines are not covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (1)
internal/net/http/client/client_test.go (1)

Line range hint 139-174:
Add test coverage for the new round tripper configuration.

The new lines handling potential errors during the transport setup are not covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between eeadafc and 0ca9ba6.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (36)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • go.mod (4 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/net/http/client/client_test.go (3 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • internal/servers/server/server_test.go (3 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/VALD_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (12)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/buildkit/syft/scanner/Dockerfile
  • example/client/go.mod
  • go.mod
  • k8s/agent/ngt/configmap.yaml
  • k8s/index/operator/deployment.yaml
  • k8s/manager/index/configmap.yaml
  • k8s/manager/index/deployment.yaml
  • versions/GO_VERSION
  • versions/VALD_VERSION
Files skipped from review as they are similar to previous changes (17)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml
  • .github/workflows/helm.yml
  • Makefile
  • Makefile.d/docker.mk
  • charts/vald-helm-operator/crds/valdrelease.yaml
  • charts/vald/values.yaml
  • dockers/agent/core/agent/Dockerfile
  • dockers/ci/base/Dockerfile
  • k8s/discoverer/configmap.yaml
  • k8s/discoverer/deployment.yaml
  • k8s/gateway/gateway/lb/configmap.yaml
  • k8s/gateway/gateway/lb/deployment.yaml
  • k8s/index/job/correction/configmap.yaml
  • k8s/index/job/creation/configmap.yaml
  • k8s/index/operator/configmap.yaml
  • k8s/operator/helm/crds/valdrelease.yaml
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional context used
GitHub Check: codecov/patch
internal/net/http/client/client.go

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

internal/servers/server/server.go

[warning] 190-193: internal/servers/server/server.go#L190-L193
Added lines #L190 - L193 were not covered by tests


[warning] 195-197: internal/servers/server/server.go#L195-L197
Added lines #L195 - L197 were not covered by tests


[warning] 200-202: internal/servers/server/server.go#L200-L202
Added lines #L200 - L202 were not covered by tests


[warning] 204-204: internal/servers/server/server.go#L204
Added line #L204 was not covered by tests

internal/config/server.go

[warning] 275-285: internal/config/server.go#L275-L285
Added lines #L275 - L285 were not covered by tests

internal/servers/server/option.go

[warning] 279-282: internal/servers/server/option.go#L279-L282
Added lines #L279 - L282 were not covered by tests


[warning] 286-290: internal/servers/server/option.go#L286-L290
Added lines #L286 - L290 were not covered by tests


[warning] 292-292: internal/servers/server/option.go#L292
Added line #L292 was not covered by tests


[warning] 294-294: internal/servers/server/option.go#L294
Added line #L294 was not covered by tests


[warning] 298-301: internal/servers/server/option.go#L298-L301
Added lines #L298 - L301 were not covered by tests


[warning] 303-304: internal/servers/server/option.go#L303-L304
Added lines #L303 - L304 were not covered by tests


[warning] 308-312: internal/servers/server/option.go#L308-L312
Added lines #L308 - L312 were not covered by tests


[warning] 314-314: internal/servers/server/option.go#L314
Added line #L314 was not covered by tests


[warning] 316-316: internal/servers/server/option.go#L316
Added line #L316 was not covered by tests


[warning] 320-324: internal/servers/server/option.go#L320-L324
Added lines #L320 - L324 were not covered by tests


[warning] 326-326: internal/servers/server/option.go#L326
Added line #L326 was not covered by tests


[warning] 328-328: internal/servers/server/option.go#L328
Added line #L328 was not covered by tests


[warning] 332-336: internal/servers/server/option.go#L332-L336
Added lines #L332 - L336 were not covered by tests


[warning] 338-338: internal/servers/server/option.go#L338
Added line #L338 was not covered by tests


[warning] 340-340: internal/servers/server/option.go#L340
Added line #L340 was not covered by tests


[warning] 344-348: internal/servers/server/option.go#L344-L348
Added lines #L344 - L348 were not covered by tests


[warning] 350-350: internal/servers/server/option.go#L350
Added line #L350 was not covered by tests


[warning] 352-352: internal/servers/server/option.go#L352
Added line #L352 was not covered by tests


[warning] 356-360: internal/servers/server/option.go#L356-L360
Added lines #L356 - L360 were not covered by tests


[warning] 362-362: internal/servers/server/option.go#L362
Added line #L362 was not covered by tests


[warning] 364-364: internal/servers/server/option.go#L364
Added line #L364 was not covered by tests


[warning] 368-372: internal/servers/server/option.go#L368-L372
Added lines #L368 - L372 were not covered by tests


[warning] 374-374: internal/servers/server/option.go#L374
Added line #L374 was not covered by tests


[warning] 376-376: internal/servers/server/option.go#L376
Added line #L376 was not covered by tests

Additional comments not posted (25)
internal/net/http/client/client.go (1)

Line range hint 66-70:
LGTM!

The changes ensure the correct transport settings are used.

Tools
GitHub Check: codecov/patch

[warning] 54-61: internal/net/http/client/client.go#L54-L61
Added lines #L54 - L61 were not covered by tests

internal/net/http/client/client_test.go (1)

91-92: LGTM!

The changes refine the comparison logic used in tests.

internal/servers/server/server.go (3)

43-43: LGTM!

The import is necessary for the new HTTP/2 support.


92-96: LGTM!

The new fields are necessary for configuring and enabling HTTP/2 support.


190-204: LGTM!

The error handling is improved, and the HTTP/2 server configuration is correctly implemented.

Tools
GitHub Check: codecov/patch

[warning] 190-193: internal/servers/server/server.go#L190-L193
Added lines #L190 - L193 were not covered by tests


[warning] 195-197: internal/servers/server/server.go#L195-L197
Added lines #L195 - L197 were not covered by tests


[warning] 200-202: internal/servers/server/server.go#L200-L202
Added lines #L200 - L202 were not covered by tests


[warning] 204-204: internal/servers/server/server.go#L204
Added line #L204 was not covered by tests

k8s/index/job/save/configmap.yaml (3)

85-94: HTTP/2 Configuration for Port 3000 Looks Good

The HTTP/2 configuration parameters are correctly added and initialized.


120-129: HTTP/2 Configuration for Port 3001 Looks Good

The HTTP/2 configuration parameters are correctly added and initialized.


156-165: HTTP/2 Configuration for Port 6060 Looks Good

The HTTP/2 configuration parameters are correctly added and initialized.

internal/config/server.go (6)

32-36: New Fields in Servers Struct Look Good

The new fields TLS and FullShutdownDuration are correctly added and initialized.


56-66: Reordering of Fields in Server Struct Looks Good

The reordering of fields, including the Port field, does not affect the underlying logic.


71-71: New HTTP2 Field in HTTP Struct Looks Good

The new HTTP2 field is correctly added and initialized.


81-91: New Fields in HTTP2 Struct Look Good

The new fields in the HTTP2 struct are correctly added and initialized.


95-99: New Fields in GRPC Struct Look Good

The new fields in the GRPC struct are correctly added and initialized.


274-286: Conditional Appending of HTTP/2 Options in Opts Method Looks Good

The changes to the Opts method correctly handle the conditional appending of HTTP/2 options.

Tools
GitHub Check: codecov/patch

[warning] 275-285: internal/config/server.go#L275-L285
Added lines #L275 - L285 were not covered by tests

internal/servers/server/option.go (9)

279-284: WithHTTP2Enabled Function Looks Good

The function correctly enables or disables HTTP/2 support based on the boolean parameter provided.

Tools
GitHub Check: codecov/patch

[warning] 279-282: internal/servers/server/option.go#L279-L282
Added lines #L279 - L282 were not covered by tests


286-296: WithHandlerLimit Function Looks Good

The function correctly sets a limit on the maximum number of handlers for the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 286-290: internal/servers/server/option.go#L286-L290
Added lines #L286 - L290 were not covered by tests


[warning] 292-292: internal/servers/server/option.go#L292
Added line #L292 was not covered by tests


[warning] 294-294: internal/servers/server/option.go#L294
Added line #L294 was not covered by tests


298-306: WithPermitProhibitedCipherSuites Function Looks Good

The function correctly configures whether to permit the use of prohibited cipher suites in the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 298-301: internal/servers/server/option.go#L298-L301
Added lines #L298 - L301 were not covered by tests


[warning] 303-304: internal/servers/server/option.go#L303-L304
Added lines #L303 - L304 were not covered by tests


308-318: WithMaxUploadBufferPerConnection Function Looks Good

The function correctly sets the maximum upload buffer size per connection.

Tools
GitHub Check: codecov/patch

[warning] 308-312: internal/servers/server/option.go#L308-L312
Added lines #L308 - L312 were not covered by tests


[warning] 314-314: internal/servers/server/option.go#L314
Added line #L314 was not covered by tests


[warning] 316-316: internal/servers/server/option.go#L316
Added line #L316 was not covered by tests


320-330: WithMaxUploadBufferPerStream Function Looks Good

The function correctly sets the maximum upload buffer size per stream.

Tools
GitHub Check: codecov/patch

[warning] 320-324: internal/servers/server/option.go#L320-L324
Added lines #L320 - L324 were not covered by tests


[warning] 326-326: internal/servers/server/option.go#L326
Added line #L326 was not covered by tests


[warning] 328-328: internal/servers/server/option.go#L328
Added line #L328 was not covered by tests


332-342: WithMaxConcurrentStreams Function Looks Good

The function correctly sets the maximum number of concurrent streams for the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 332-336: internal/servers/server/option.go#L332-L336
Added lines #L332 - L336 were not covered by tests


[warning] 338-338: internal/servers/server/option.go#L338
Added line #L338 was not covered by tests


[warning] 340-340: internal/servers/server/option.go#L340
Added line #L340 was not covered by tests


344-354: WithMaxDecoderHeaderTableSize Function Looks Good

The function correctly sets the maximum size of the decoder header table for the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 344-348: internal/servers/server/option.go#L344-L348
Added lines #L344 - L348 were not covered by tests


[warning] 350-350: internal/servers/server/option.go#L350
Added line #L350 was not covered by tests


[warning] 352-352: internal/servers/server/option.go#L352
Added line #L352 was not covered by tests


356-366: WithMaxEncoderHeaderTableSize Function Looks Good

The function correctly sets the maximum size of the encoder header table for the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 356-360: internal/servers/server/option.go#L356-L360
Added lines #L356 - L360 were not covered by tests


[warning] 362-362: internal/servers/server/option.go#L362
Added line #L362 was not covered by tests


[warning] 364-364: internal/servers/server/option.go#L364
Added line #L364 was not covered by tests


368-378: WithMaxReadFrameSize Function Looks Good

The function correctly sets the maximum frame size that can be read by the HTTP/2 server.

Tools
GitHub Check: codecov/patch

[warning] 368-372: internal/servers/server/option.go#L368-L372
Added lines #L368 - L372 were not covered by tests


[warning] 374-374: internal/servers/server/option.go#L374
Added line #L374 was not covered by tests


[warning] 376-376: internal/servers/server/option.go#L376
Added line #L376 was not covered by tests

internal/servers/server/server_test.go (2)

506-510: Ensure proper initialization and usage of new fields.

The new fields h2srv and enableH2 should be properly initialized and used within the test logic to verify HTTP/2 support.


667-671: Ensure proper initialization and usage of new fields.

The new fields h2srv and enableH2 should be properly initialized and used within the test logic to verify HTTP/2 support.

internal/net/http/client/client.go Outdated Show resolved Hide resolved
internal/servers/server/server.go Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 6, 2024
@kpango kpango force-pushed the feature/internal/enable-http2-features branch from 0ca9ba6 to d960a99 Compare August 6, 2024 23:26
deepsource-autofix bot added a commit that referenced this pull request Aug 6, 2024
This commit fixes the style issues introduced in d960a99 according to the output
from Gofumpt and Prettier.

Details: #2572
@kpango kpango force-pushed the feature/internal/enable-http2-features branch from 8cb3b95 to c29aab2 Compare August 6, 2024 23:30
deepsource-autofix bot added a commit that referenced this pull request Aug 6, 2024
This commit fixes the style issues introduced in c29aab2 according to the output
from Gofumpt and Prettier.

Details: #2572
@kpango kpango force-pushed the feature/internal/enable-http2-features branch from 675654a to c6fd92a Compare August 6, 2024 23:32
Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango force-pushed the feature/internal/enable-http2-features branch from c6fd92a to 1c30a54 Compare August 6, 2024 23:33
@kpango kpango requested review from hlts2 and datelier August 6, 2024 23:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0ca9ba6 and 1c30a54.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (38)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • example/client/go.mod.default (1 hunks)
  • go.mod (5 hunks)
  • hack/go.mod.default (1 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/net/http/client/client_test.go (2 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • internal/servers/server/server_test.go (3 hunks)
  • internal/test/data/hdf5/hdf5.go (1 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (12)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/agent/core/agent/Dockerfile
  • dockers/buildkit/syft/scanner/Dockerfile
  • dockers/ci/base/Dockerfile
  • example/client/go.mod
  • example/client/go.mod.default
  • go.mod
  • hack/go.mod.default
  • k8s/agent/ngt/configmap.yaml
  • versions/GO_VERSION
Files skipped from review as they are similar to previous changes (23)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml
  • .github/workflows/helm.yml
  • Makefile
  • Makefile.d/docker.mk
  • charts/vald-helm-operator/crds/valdrelease.yaml
  • charts/vald/values.yaml
  • internal/config/server.go
  • internal/net/http/client/client.go
  • internal/net/http/client/client_test.go
  • internal/servers/server/option.go
  • internal/servers/server/server.go
  • internal/servers/server/server_test.go
  • k8s/discoverer/configmap.yaml
  • k8s/discoverer/deployment.yaml
  • k8s/gateway/gateway/lb/configmap.yaml
  • k8s/index/job/correction/configmap.yaml
  • k8s/index/job/creation/configmap.yaml
  • k8s/index/job/save/configmap.yaml
  • k8s/index/operator/configmap.yaml
  • k8s/index/operator/deployment.yaml
  • k8s/manager/index/configmap.yaml
  • k8s/manager/index/deployment.yaml
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional comments not posted (29)
k8s/gateway/gateway/lb/deployment.yaml (1)

48-48: Ensure the deployment uses the latest configuration.

The checksum value for the configmap annotation has been updated. Ensure that the deployment is refreshed to use the latest configuration settings.

k8s/operator/helm/crds/valdrelease.yaml (28)

585-605: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


702-722: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


831-851: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


1041-1061: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


1546-1566: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


1663-1683: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


1792-1812: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


2002-2022: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


2481-2501: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


2598-2618: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


2727-2747: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


2937-2957: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


3432-3452: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


3549-3569: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


3678-3698: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


3888-3908: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


4887-4907: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


5004-5024: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


5133-5153: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


5343-5363: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


6315-6335: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


6432-6452: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


6561-6581: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


6771-6791: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


7463-7483: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


7580-7600: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


7709-7729: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The http2 property and its sub-properties are correctly defined. Ensure that similar definitions are consistent across all sections.


7919-7939: Ensure Consistency and Correctness in HTTP/2 Schema Definitions

The `http2

internal/test/data/hdf5/hdf5.go Show resolved Hide resolved
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Aug 6, 2024

[WARNING:CONFLICT] You may require to fix the conflict. Please check.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0ca9ba6 and 1c30a54.

Files ignored due to path filters (2)
  • example/client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
Files selected for processing (38)
  • .github/ISSUE_TEMPLATE/bug_report.md (1 hunks)
  • .github/ISSUE_TEMPLATE/security_issue_report.md (1 hunks)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml (1 hunks)
  • .github/workflows/helm.yml (1 hunks)
  • Makefile (2 hunks)
  • Makefile.d/docker.mk (3 hunks)
  • charts/vald-helm-operator/crds/valdrelease.yaml (52 hunks)
  • charts/vald/values.yaml (4 hunks)
  • dockers/agent/core/agent/Dockerfile (1 hunks)
  • dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
  • dockers/ci/base/Dockerfile (1 hunks)
  • example/client/go.mod (2 hunks)
  • example/client/go.mod.default (1 hunks)
  • go.mod (5 hunks)
  • hack/go.mod.default (1 hunks)
  • internal/config/server.go (4 hunks)
  • internal/net/http/client/client.go (1 hunks)
  • internal/net/http/client/client_test.go (2 hunks)
  • internal/servers/server/option.go (2 hunks)
  • internal/servers/server/server.go (3 hunks)
  • internal/servers/server/server_test.go (3 hunks)
  • internal/test/data/hdf5/hdf5.go (1 hunks)
  • k8s/agent/ngt/configmap.yaml (3 hunks)
  • k8s/discoverer/configmap.yaml (3 hunks)
  • k8s/discoverer/deployment.yaml (1 hunks)
  • k8s/gateway/gateway/lb/configmap.yaml (3 hunks)
  • k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
  • k8s/index/job/correction/configmap.yaml (3 hunks)
  • k8s/index/job/creation/configmap.yaml (3 hunks)
  • k8s/index/job/save/configmap.yaml (3 hunks)
  • k8s/index/operator/configmap.yaml (1 hunks)
  • k8s/index/operator/deployment.yaml (1 hunks)
  • k8s/manager/index/configmap.yaml (3 hunks)
  • k8s/manager/index/deployment.yaml (1 hunks)
  • k8s/operator/helm/crds/valdrelease.yaml (52 hunks)
  • versions/GO_VERSION (1 hunks)
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT (1 hunks)
Files skipped from review due to trivial changes (11)
  • .github/ISSUE_TEMPLATE/bug_report.md
  • .github/ISSUE_TEMPLATE/security_issue_report.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • dockers/agent/core/agent/Dockerfile
  • dockers/buildkit/syft/scanner/Dockerfile
  • dockers/ci/base/Dockerfile
  • example/client/go.mod
  • example/client/go.mod.default
  • go.mod
  • hack/go.mod.default
  • versions/GO_VERSION
Files skipped from review as they are similar to previous changes (22)
  • .github/workflows/dockers-buildkit-syft-scanner-image.yaml
  • .github/workflows/helm.yml
  • Makefile
  • Makefile.d/docker.mk
  • charts/vald-helm-operator/crds/valdrelease.yaml
  • internal/config/server.go
  • internal/net/http/client/client.go
  • internal/net/http/client/client_test.go
  • internal/servers/server/option.go
  • internal/servers/server/server.go
  • internal/servers/server/server_test.go
  • k8s/agent/ngt/configmap.yaml
  • k8s/discoverer/configmap.yaml
  • k8s/discoverer/deployment.yaml
  • k8s/gateway/gateway/lb/configmap.yaml
  • k8s/index/job/correction/configmap.yaml
  • k8s/index/job/creation/configmap.yaml
  • k8s/index/job/save/configmap.yaml
  • k8s/index/operator/configmap.yaml
  • k8s/manager/index/configmap.yaml
  • k8s/manager/index/deployment.yaml
  • versions/actions/ACTIONS_UPLOAD_ARTIFACT
Additional comments not posted (35)
k8s/index/operator/deployment.yaml (1)

49-49: Update checksum/configmap annotation

The checksum/configmap annotation has been updated to reflect changes in the configuration map. Ensure that the new configuration map is correctly applied and tested.

Verification successful

Update checksum/configmap annotation

The checksum/configmap annotation has been updated and the deployment correctly references the updated ConfigMap. The new configuration map is correctly applied.

  • k8s/index/operator/deployment.yaml references vald-index-operator-config correctly.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new checksum/configmap annotation is correctly applied.

# Test: Check the deployment configuration. Expect: The new checksum value should be present.
rg --type yaml -A 2 'checksum/configmap: eae6b3eac702f445f9b5a0d1495af9917479303b103f986c10b5b3db9d749086'

Length of output: 401


Script:

#!/bin/bash
# Description: Verify the new checksum/configmap annotation is correctly applied.

# Check for the ConfigMap definition to ensure it matches the new checksum.
rg --type yaml 'kind: ConfigMap' -A 10

# Verify if the deployment is referencing the correct ConfigMap.
rg --type yaml 'kind: Deployment' -A 10

Length of output: 63354


Script:

#!/bin/bash
# Description: Verify the deployment is correctly referencing the updated ConfigMap.

# Check the deployment configuration in k8s/index/operator/deployment.yaml to ensure it references the vald-index-operator-config ConfigMap.
rg --type yaml 'name: vald-index-operator-config' -A 10 k8s/index/operator/deployment.yaml

Length of output: 659

internal/test/data/hdf5/hdf5.go (1)

212-215: Enhance HTTP client initialization

The downloadFile function now includes options to force HTTP/2 and enable keep-alives. This enhances connection handling and performance.

k8s/gateway/gateway/lb/deployment.yaml (1)

48-48: Update checksum/configmap annotation

The checksum/configmap annotation has been updated to reflect changes in the configuration map. Ensure that the new configuration map is correctly applied and tested.

Verification successful

The new checksum/configmap annotation is correctly applied in the k8s/gateway/gateway/lb/deployment.yaml file. However, please ensure that the new configuration map is correctly applied and tested in the deployment.

  • The checksum value 92b60d09b5e957c16d7a665138f8ddd8765c38d47df0740b05e55871ddd20cf4 is present in the deployment file.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new checksum/configmap annotation is correctly applied.

# Test: Check the deployment configuration. Expect: The new checksum value should be present.
rg --type yaml -A 2 'checksum/configmap: 92b60d09b5e957c16d7a665138f8ddd8765c38d47df0740b05e55871ddd20cf4'

Length of output: 386

charts/vald/values.yaml (4)

388-407: LGTM! Verify consistency across all sections.

The HTTP/2 configuration parameters are well-defined and cover necessary aspects. Ensure that similar parameters are consistently defined across all sections.


508-526: LGTM! Verify consistency across all sections.

The HTTP/2 configuration parameters are well-defined and cover necessary aspects. Ensure that similar parameters are consistently defined across all sections.


601-619: LGTM! Verify consistency across all sections.

The HTTP/2 configuration parameters are well-defined and cover necessary aspects. Ensure that similar parameters are consistently defined across all sections.


97-125: LGTM! Verify consistency across all sections.

The HTTP/2 configuration parameters are well-defined and cover necessary aspects. Ensure that similar parameters are consistently defined across all sections.

Verification successful

HTTP/2 Configuration Consistency Verified

The HTTP/2 configuration parameters are consistently defined across various sections in multiple files, ensuring uniform behavior across the system.

  • k8s/agent/ngt/configmap.yaml
  • k8s/manager/index/configmap.yaml
  • k8s/index/job/correction/configmap.yaml
  • k8s/index/job/creation/configmap.yaml
  • k8s/index/job/save/configmap.yaml
  • k8s/operator/helm/crds/valdrelease.yaml
  • k8s/gateway/gateway/lb/configmap.yaml
  • k8s/discoverer/configmap.yaml
  • charts/vald/values.yaml
  • charts/vald-helm-operator/crds/valdrelease.yaml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of HTTP/2 configuration parameters across all sections.

# Test: Search for HTTP/2 configuration sections. Expect: Consistent parameter definitions.
rg --type yaml -A 10 'http2:'

Length of output: 171754

k8s/operator/helm/crds/valdrelease.yaml (28)

585-605: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


702-722: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


831-851: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


1041-1061: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


1546-1566: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


1663-1683: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


1792-1812: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


2002-2022: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


2481-2501: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


2598-2618: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


2727-2747: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


2937-2957: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


3432-3452: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


3549-3569: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


3678-3698: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


3888-3908: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


4887-4907: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


5004-5024: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


5133-5153: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


5343-5363: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


6315-6335: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


6432-6452: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


6561-6581: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


6771-6791: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


7463-7483: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


7580-7600: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


7709-7729: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.


7919-7939: Verify the correctness of the added properties.

The http2 object includes properties with various types. Ensure that these properties and their types align with the intended schema and functionality.

@datelier datelier merged commit 3457a5b into main Aug 7, 2024
183 of 184 checks passed
@datelier datelier deleted the feature/internal/enable-http2-features branch August 7, 2024 01:02
vdaas-ci pushed a commit that referenced this pull request Aug 7, 2024
kpango added a commit that referenced this pull request Aug 7, 2024
Signed-off-by: kpango <kpango@vdaas.org>
Co-authored-by: Yusuke Kato <kpango@vdaas.org>
kpango added a commit that referenced this pull request Aug 8, 2024
fix: git add chart directory for release (#2356) (#2357)
[patch] release v1.7.11 (#2358)
:bookmark: :robot: Release v1.7.11 (#2360)
Change docker scan timeout longer (#2363) (#2364)
refactor code using golangci-lint (#2362) (#2365)
Create SECURITY.md (#2367) (#2368)
add commit hash build image (#2359) (#2371)
update docker build target platform selection rules (#2370) (#2374)
Make agent export index metrics to Pod k8s resource (#2319) (#2372)
backport ci deps others (#2386)
Update workflow to release readreplica chart (#2383) (#2387)
:green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389)
Delete unnecessary code for mirror (#2366) (#2391)
change JP logo to EN logo (#2369) (#2392)
Add rotate-all option to rotator (#2305) (#2393)
fix: build error of internal kvs test (#2396) (#2398)
Resolve kvs already closed before last saving (#2390) (#2394)
:robot: Update license headers / Format Go codes and YAML files (#2397) (#2400)
create continous benchmark doc (#2352) (#2395)
fix: disable protobuf dispatch for client (#2401) (#2403)
update deps (#2404) (#2405)
[patch] release v1.7.12 (#2406)
:bookmark: :robot: Release v1.7.12 (#2408)
:pencil: Fix typo of file name (#2413) (#2415)
Fix agent-faiss build failed (#2418) (#2419)
Add tests for index information export (#2412) (#2414)
Fix the logic to determine docker image (#2410) (#2420)
Update build rule for nightly image (#2421) (#2422)
Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425)
Add `index-operator` template implementation (#2375) (#2424)
fix: typo of execution rule (#2426) (#2427)
Backport Flush API (#2434)
update deps & add validation for Flush API when agent is Read Only (#2433) (#2436)
docs: add hrichiksite as a contributor for doc (#2441) (#2442)
fix: bugfix version update for docker build (#2445) (#2446)
Fix index job logic to pass DNS A record (#2438) (#2448)
Added snapshot timestamp annotations to read replica agent (#2428) (#2443)
Fix operator-sdk version (#2447) (#2449)
add file name lint (#2417) (#2450)
fix: add extra option for ci-container build (#2451) (#2452)
Add base of benchmark operator dashboard (#2430) (#2453)
Implement index operator logic for read replica rotation (#2444) (#2456)
add inner product distance type for ngt (#2454) (#2458)
Fix e2e for read replica and add e2e for index operator (#2455) (#2459)
Add unit tests for index operator (#2460) (#2461)
Bugfix recreate benchmark job when operator reboot (#2463) (#2464)
Refactor k8s types (#2462) (#2465)
:robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469)
Fix workflow trigger for backport pr creation (#2471) (#2472)
Automatically add backport main label for release-pr (#2473) (#2475)
update deps (#2468) (#2476)
Implement client metrics interceptor for continuous benchmark job (#2477) (#2480)
:chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483)
Update continuous benchmark docs (#2485) (#2486)
Sync release/v1.7 to main (#2495)
add read replica and rotator docs (#2497) (#2499)
add reviewer guideline (#2507) (#2508)
update large top-K ratio handling logic (#2509) (#2511)
Change default image tag from latest to nightly (#2516) (#2518)
Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520)
add faiss in values.yaml & valdrelease.yaml (#2514) (#2519)
capitalize faq (#2512) (#2522)
Backport docs updates to release/v1.7 (#2521)
[CI] Add workflow to synchronize ubuntu base image (#2526) (#2527)
fix: update schedule (#2528) (#2530)
refactor index manager service add index service API to expose index informations (#2525) (#2532)
fix conflict bug (#2537)
fix: make format (#2534) (#2540)
Backport PR #2542, #2538 to release/v1.7 (#2543)
fix: add checkout option (#2545) (#2546)
Implement ngt Statistics API (#2539) (#2547)
Add workflow to check git conflict for backport PR (#2548) (#2550)
[create-pull-request] automated change (#2552) (#2556)
Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557)
Backport #2559 (#2560)
[BUGFIX] index correction process (#2565) (#2566)
change external docker image reference to ghcr.io registry (#2567) (#2568)
[patch] Release v1.7.13 (#2569)
:bookmark: :robot: Release v1.7.13 (#2570)
add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575)

Signed-off-by: kpango <kpango@vdaas.org>
kpango added a commit that referenced this pull request Aug 8, 2024
fix: git add chart directory for release (#2356) (#2357)
[patch] release v1.7.11 (#2358)
:bookmark: :robot: Release v1.7.11 (#2360)
Change docker scan timeout longer (#2363) (#2364)
refactor code using golangci-lint (#2362) (#2365)
Create SECURITY.md (#2367) (#2368)
add commit hash build image (#2359) (#2371)
update docker build target platform selection rules (#2370) (#2374)
Make agent export index metrics to Pod k8s resource (#2319) (#2372)
backport ci deps others (#2386)
Update workflow to release readreplica chart (#2383) (#2387)
:green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389)
Delete unnecessary code for mirror (#2366) (#2391)
change JP logo to EN logo (#2369) (#2392)
Add rotate-all option to rotator (#2305) (#2393)
fix: build error of internal kvs test (#2396) (#2398)
Resolve kvs already closed before last saving (#2390) (#2394)
:robot: Update license headers / Format Go codes and YAML files (#2397) (#2400)
create continous benchmark doc (#2352) (#2395)
fix: disable protobuf dispatch for client (#2401) (#2403)
update deps (#2404) (#2405)
[patch] release v1.7.12 (#2406)
:bookmark: :robot: Release v1.7.12 (#2408)
:pencil: Fix typo of file name (#2413) (#2415)
Fix agent-faiss build failed (#2418) (#2419)
Add tests for index information export (#2412) (#2414)
Fix the logic to determine docker image (#2410) (#2420)
Update build rule for nightly image (#2421) (#2422)
Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425)
Add `index-operator` template implementation (#2375) (#2424)
fix: typo of execution rule (#2426) (#2427)
Backport Flush API (#2434)
update deps & add validation for Flush API when agent is Read Only (#2433) (#2436)
docs: add hrichiksite as a contributor for doc (#2441) (#2442)
fix: bugfix version update for docker build (#2445) (#2446)
Fix index job logic to pass DNS A record (#2438) (#2448)
Added snapshot timestamp annotations to read replica agent (#2428) (#2443)
Fix operator-sdk version (#2447) (#2449)
add file name lint (#2417) (#2450)
fix: add extra option for ci-container build (#2451) (#2452)
Add base of benchmark operator dashboard (#2430) (#2453)
Implement index operator logic for read replica rotation (#2444) (#2456)
add inner product distance type for ngt (#2454) (#2458)
Fix e2e for read replica and add e2e for index operator (#2455) (#2459)
Add unit tests for index operator (#2460) (#2461)
Bugfix recreate benchmark job when operator reboot (#2463) (#2464)
Refactor k8s types (#2462) (#2465)
:robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469)
Fix workflow trigger for backport pr creation (#2471) (#2472)
Automatically add backport main label for release-pr (#2473) (#2475)
update deps (#2468) (#2476)
Implement client metrics interceptor for continuous benchmark job (#2477) (#2480)
:chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483)
Update continuous benchmark docs (#2485) (#2486)
Sync release/v1.7 to main (#2495)
add read replica and rotator docs (#2497) (#2499)
add reviewer guideline (#2507) (#2508)
update large top-K ratio handling logic (#2509) (#2511)
Change default image tag from latest to nightly (#2516) (#2518)
Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520)
add faiss in values.yaml & valdrelease.yaml (#2514) (#2519)
capitalize faq (#2512) (#2522)
Backport docs updates to release/v1.7 (#2521)
[CI] Add workflow to synchronize ubuntu base image (#2526) (#2527)
fix: update schedule (#2528) (#2530)
refactor index manager service add index service API to expose index informations (#2525) (#2532)
fix conflict bug (#2537)
fix: make format (#2534) (#2540)
Backport PR #2542, #2538 to release/v1.7 (#2543)
fix: add checkout option (#2545) (#2546)
Implement ngt Statistics API (#2539) (#2547)
Add workflow to check git conflict for backport PR (#2548) (#2550)
[create-pull-request] automated change (#2552) (#2556)
Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557)
Backport #2559 (#2560)
[BUGFIX] index correction process (#2565) (#2566)
change external docker image reference to ghcr.io registry (#2567) (#2568)
[patch] Release v1.7.13 (#2569)
:bookmark: :robot: Release v1.7.13 (#2570)
add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575)

Signed-off-by: kpango <kpango@vdaas.org>
kpango added a commit that referenced this pull request Aug 8, 2024
fix: git add chart directory for release (#2356) (#2357)
[patch] release v1.7.11 (#2358)
:bookmark: :robot: Release v1.7.11 (#2360)
Change docker scan timeout longer (#2363) (#2364)
refactor code using golangci-lint (#2362) (#2365)
Create SECURITY.md (#2367) (#2368)
add commit hash build image (#2359) (#2371)
update docker build target platform selection rules (#2370) (#2374)
Make agent export index metrics to Pod k8s resource (#2319) (#2372)
backport ci deps others (#2386)
Update workflow to release readreplica chart (#2383) (#2387)
:green_heart: :recycle: Add Con-Bench helm chart to the Vald charts (#2388) (#2389)
Delete unnecessary code for mirror (#2366) (#2391)
change JP logo to EN logo (#2369) (#2392)
Add rotate-all option to rotator (#2305) (#2393)
fix: build error of internal kvs test (#2396) (#2398)
Resolve kvs already closed before last saving (#2390) (#2394)
:robot: Update license headers / Format Go codes and YAML files (#2397) (#2400)
create continous benchmark doc (#2352) (#2395)
fix: disable protobuf dispatch for client (#2401) (#2403)
update deps (#2404) (#2405)
[patch] release v1.7.12 (#2406)
:bookmark: :robot: Release v1.7.12 (#2408)
:pencil: Fix typo of file name (#2413) (#2415)
Fix agent-faiss build failed (#2418) (#2419)
Add tests for index information export (#2412) (#2414)
Fix the logic to determine docker image (#2410) (#2420)
Update build rule for nightly image (#2421) (#2422)
Fix output settings to determine-docker-image-tag action and release branch build tag name (#2423) (#2425)
Add `index-operator` template implementation (#2375) (#2424)
fix: typo of execution rule (#2426) (#2427)
Backport Flush API (#2434)
update deps & add validation for Flush API when agent is Read Only (#2433) (#2436)
docs: add hrichiksite as a contributor for doc (#2441) (#2442)
fix: bugfix version update for docker build (#2445) (#2446)
Fix index job logic to pass DNS A record (#2438) (#2448)
Added snapshot timestamp annotations to read replica agent (#2428) (#2443)
Fix operator-sdk version (#2447) (#2449)
add file name lint (#2417) (#2450)
fix: add extra option for ci-container build (#2451) (#2452)
Add base of benchmark operator dashboard (#2430) (#2453)
Implement index operator logic for read replica rotation (#2444) (#2456)
add inner product distance type for ngt (#2454) (#2458)
Fix e2e for read replica and add e2e for index operator (#2455) (#2459)
Add unit tests for index operator (#2460) (#2461)
Bugfix recreate benchmark job when operator reboot (#2463) (#2464)
Refactor k8s types (#2462) (#2465)
:robot: Automatically update PULL_REQUEST_TEMPLATE and ISSUE_TEMPLATE (#2457) (#2469)
Fix workflow trigger for backport pr creation (#2471) (#2472)
Automatically add backport main label for release-pr (#2473) (#2475)
update deps (#2468) (#2476)
Implement client metrics interceptor for continuous benchmark job (#2477) (#2480)
:chart_with_upwards_trend: Add client metrics panels for continuous benchmark job (#2481) (#2483)
Update continuous benchmark docs (#2485) (#2486)
Sync release/v1.7 to main (#2495)
add read replica and rotator docs (#2497) (#2499)
add reviewer guideline (#2507) (#2508)
update large top-K ratio handling logic (#2509) (#2511)
Change default image tag from latest to nightly (#2516) (#2518)
Bugfix that caused an error when argument has 3 or more nil arguments (#2517) (#2520)
add faiss in values.yaml & valdrelease.yaml (#2514) (#2519)
capitalize faq (#2512) (#2522)
Backport docs updates to release/v1.7 (#2521)
[CI] Add workflow to synchronize ubuntu base image (#2526) (#2527)
fix: update schedule (#2528) (#2530)
refactor index manager service add index service API to expose index informations (#2525) (#2532)
fix conflict bug (#2537)
fix: make format (#2534) (#2540)
Backport PR #2542, #2538 to release/v1.7 (#2543)
fix: add checkout option (#2545) (#2546)
Implement ngt Statistics API (#2539) (#2547)
Add workflow to check git conflict for backport PR (#2548) (#2550)
[create-pull-request] automated change (#2552) (#2556)
Update dependencies, C++ standard, and improve Dockerfiles for better build systems and localization (#2549) (#2557)
Backport #2559 (#2560)
[BUGFIX] index correction process (#2565) (#2566)
change external docker image reference to ghcr.io registry (#2567) (#2568)
[patch] Release v1.7.13 (#2569)
:bookmark: :robot: Release v1.7.13 (#2570)
add HTTP2 support for http.Client and Vald HTTP Server (#2572) (#2575)

Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango mentioned this pull request Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants