Enable retain line breaks #2851
Conversation
📝 WalkthroughWalkthroughThis change introduces a comprehensive set of new GitHub Actions workflows for building various Docker images. Each workflow is configured with event triggers for pushes, tags, and pull requests, and reuses a common build job defined in Changes
Sequence Diagram(s)sequenceDiagram
participant E as Git Event
participant W as Docker Build Workflow
participant C as Common Build Workflow (_docker-image.yaml)
participant D as Docker Build Service
E->>W: Trigger event (push/PR/tag)
W->>C: Invoke common workflow with target parameter
C->>D: Build Docker image for specific target
D-->>C: Return build result
C-->>W: Report status
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[CHATOPS:HELP] ChatOps commands.
|
github-actions
bot
added
type/ci
area/helm
size/S
type/bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
charts/vald-benchmark-operator/schemas/scenario-values.yaml (1)
39-46: 🛠️ Refactor suggestionAdd validation to ensure end is greater than start.
The range schema should validate that the end value is greater than the start value to prevent invalid ranges.
- # @schema {"name": "dataset.range", "type": "object", "required": ["start", "end"]} + # @schema {"name": "dataset.range", "type": "object", "required": ["start", "end"], "additionalProperties": false} range: - # @schema {"name": "dataset.range.start", "type": "integer", "minimum": 1} + # @schema {"name": "dataset.range.start", "type": "integer", "minimum": 1, "maximum": {"$ref": "#/dataset/range/end"}} start: 1 - # @schema {"name": "dataset.range.end", "type": "integer", "minimum": 1} + # @schema {"name": "dataset.range.end", "type": "integer", "minimum": {"$ref": "#/dataset/range/start"}} end: 1000.github/workflows/dockers-gateway-filter-image.yaml (1)
144-256:⚠️ Potential issueSecurity concern with
pull_request_targetevent.The
pull_request_targetevent runs workflows with repository secrets using the base branch's code, but checks out PR code, which could expose secrets to untrusted PR code. This is risky when combined with path filters, as malicious PRs could modify workflow files.Consider one of these safer alternatives:
- Remove the
pull_request_targetevent and use onlypull_requestfor untrusted PRs.- If repository secrets are needed, add explicit conditions to ensure the PR is from a trusted source:
pull_request_target: + if: github.event.pull_request.head.repo.full_name == github.repository paths:
🧹 Nitpick comments (14)
charts/vald/values.yaml (3)
907-979: Consider enabling observability features by default.The observability features are currently disabled by default (enabled: false). Consider enabling them by default to ensure better monitoring and troubleshooting capabilities in production environments.
observability: - enabled: false + enabled: true
1091-1096: Consider adding memory-based HPA metrics.The HPA configuration only uses CPU utilization. Consider adding memory-based metrics for more comprehensive scaling decisions.
hpa: enabled: true targetCPUUtilizationPercentage: 80 + targetMemoryUtilizationPercentage: 80
214-225: Review gRPC keepalive settings for production use.The gRPC keepalive settings need careful consideration for production:
- time: "3h" seems too long for keepalive time
- timeout: "60s" might be too long for timeout
- min_time: "10m" could be reduced
time: "3h" - timeout: "60s" + timeout: "20s" - min_time: "10m" + min_time: "5m".github/workflows/dockers-binfmt-image.yaml (1)
21-22: Consider optimizing the schedule frequency.Running builds every hour (
0 * * * *) might be excessive for base images that don't change frequently. Consider reducing the frequency to daily or weekly builds to optimize resource usage.schedule: - - cron: 0 * * * * + - cron: 0 0 * * * # Run daily at midnight.github/workflows/dockers-buildbase-image.yaml (1)
21-32: Consider using environment files for version patterns.The branch and tag patterns are repeated across multiple workflow files. Consider using environment files to store these patterns, making them easier to maintain and update.
Create a
.github/workflow-envfile:+# .github/workflow-env +RELEASE_BRANCH_PATTERN="release/v*.*" +EXCLUDE_BRANCH_PATTERN="!release/v*.*.*" +TAG_PATTERNS="*.*.* *.*.* v*.*.* v*.*.*-*"Then modify the workflow:
- branches: - - main - - release/v*.* - - "!release/v*.*.*" - tags: - - "*.*.*" - - "*.*.*-*" - - v*.*.* - - v*.*.*-* + branches: ${{ vars.RELEASE_BRANCH_PATTERNS }} + tags: ${{ vars.TAG_PATTERNS }}.github/workflows/dockers-buildkit-syft-scanner-image.yaml (1)
21-22: Review scheduled job frequency.The workflow runs hourly which might be too frequent for a Docker image build. Consider reducing the frequency unless there's a specific requirement.
- - cron: 0 * * * * + - cron: 0 0 * * * # Daily at midnight.github/workflows/dockers-example-client-image.yaml (1)
64-68: Consider specifying platforms for cross-architecture support.Unlike other workflows, this one doesn't specify target platforms. Consider adding platform support if the example client should work across different architectures.
with: target: example-client + platforms: linux/amd64,linux/arm64 secrets: inherit.github/workflows/dockers-agent-image.yaml (1)
41-49: Consider versioning Rust dependencies separately.The workflow triggers on changes to Rust-related files. Consider separating Rust dependency versioning into a dedicated workflow to avoid unnecessary builds.
Create a new workflow specifically for Rust dependency updates and remove these paths from this workflow's triggers.
.github/workflows/dockers-helm-operator-image.yaml (1)
38-43: Consider using composite paths for Helm charts.The workflow lists multiple Helm chart paths individually. Consider using a composite path pattern to reduce maintenance overhead.
- - charts/vald-helm-operator/Chart.yaml - - charts/vald-helm-operator/templates/** - - charts/vald-helm-operator/values.yaml - - charts/vald/Chart.yaml - - charts/vald/templates/** - - charts/vald/values.yaml + - charts/**/Chart.yaml + - charts/**/templates/** + - charts/**/values.yamlcharts/vald-benchmark-operator/schemas/scenario-values.yaml (1)
29-31: Consider adding pattern validation for dataset name.The dataset name is currently limited to an enum of "original" and "fashion-mnist". Consider using a pattern validation to allow for future dataset names without schema changes.
- # @schema {"name": "dataset.name", "type": "string", "enum": ["original", "fashion-mnist"] } + # @schema {"name": "dataset.name", "type": "string", "pattern": "^[a-z0-9-]+$", "minLength": 1}.github/workflows/dockers-loadtest-image.yaml (1)
214-216: Consider platform-specific build configurations.The workflow currently specifies only
linux/amd64. Consider adding support for multiple platforms to improve compatibility.with: target: loadtest - platforms: linux/amd64 + platforms: linux/amd64,linux/arm64.github/workflows/dockers-index-deletion-image.yaml (1)
33-34: Consider adding more test file patterns to exclusions.The current exclusions only cover
*_mock.goand*_test.go. Consider adding other common test-related patterns.- - "!**/*_mock.go" - - "!**/*_test.go" + - "!**/*_mock.go" + - "!**/*_test.go" + - "!**/*_fake.go" + - "!**/*_fixtures.go" + - "!**/*_testutil.go" + - "!test/**"Also applies to: 138-139
.github/workflows/dockers-index-correction-image.yaml (1)
17-18: Consider adding validation for generated files.The comment indicates this is a generated file, but there's no automated check to ensure it hasn't been manually modified.
Consider adding a pre-commit hook or CI check that validates generated files match their source:
#!/bin/bash # Generate a checksum file during generation echo "# Add to hack/docker/gen/main.go" echo 'sha256sum .github/workflows/dockers-*.yaml > .github/workflows/.checksums' # Verify checksums in CI echo "# Add to CI workflow" echo 'sha256sum -c .github/workflows/.checksums'.github/workflows/dockers-discoverer-k8s-image.yaml (1)
27-30: Consider adding pre-release tag pattern.The tag patterns might miss pre-release versions that use different formats.
- "*.*.*" - "*.*.*-*" - v*.*.* - v*.*.*-* + - "*.*.*-alpha*" + - "*.*.*-beta*" + - "*.*.*-rc*" + - v*.*.*-alpha* + - v*.*.*-beta* + - v*.*.*-rc*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.github/workflows/dockers-agent-faiss-image.yaml(1 hunks).github/workflows/dockers-agent-image.yaml(1 hunks).github/workflows/dockers-agent-ngt-image.yaml(1 hunks).github/workflows/dockers-agent-sidecar-image.yaml(1 hunks).github/workflows/dockers-benchmark-job-image.yaml(1 hunks).github/workflows/dockers-benchmark-operator-image.yaml(1 hunks).github/workflows/dockers-binfmt-image.yaml(1 hunks).github/workflows/dockers-buildbase-image.yaml(1 hunks).github/workflows/dockers-buildkit-image.yaml(1 hunks).github/workflows/dockers-buildkit-syft-scanner-image.yaml(1 hunks).github/workflows/dockers-ci-container-image.yaml(1 hunks).github/workflows/dockers-dev-container-image.yaml(1 hunks).github/workflows/dockers-discoverer-k8s-image.yaml(1 hunks).github/workflows/dockers-example-client-image.yaml(1 hunks).github/workflows/dockers-gateway-filter-image.yaml(1 hunks).github/workflows/dockers-gateway-lb-image.yaml(1 hunks).github/workflows/dockers-gateway-mirror-image.yaml(1 hunks).github/workflows/dockers-helm-operator-image.yaml(1 hunks).github/workflows/dockers-index-correction-image.yaml(1 hunks).github/workflows/dockers-index-creation-image.yaml(1 hunks).github/workflows/dockers-index-deletion-image.yaml(1 hunks).github/workflows/dockers-index-operator-image.yaml(1 hunks).github/workflows/dockers-index-save-image.yaml(1 hunks).github/workflows/dockers-loadtest-image.yaml(1 hunks).github/workflows/dockers-manager-index-image.yaml(1 hunks).github/workflows/dockers-readreplica-rotate-image.yaml(1 hunks).yamlfmt(1 hunks)charts/vald-benchmark-operator/schemas/job-values.yaml(1 hunks)charts/vald-benchmark-operator/schemas/scenario-values.yaml(1 hunks)charts/vald-benchmark-operator/values.yaml(1 hunks)charts/vald-helm-operator/values.yaml(1 hunks)charts/vald/schemas/mirror-target-values.yaml(1 hunks)charts/vald/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .yamlfmt
- charts/vald-helm-operator/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms (180)
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
- GitHub Check: build / build
🔇 Additional comments (35)
charts/vald/values.yaml (6)
1-15: LGTM! Well-structured license header.The Apache 2.0 license header is properly included with correct copyright dates.
16-22: LGTM! Well-documented schema definition.The schema definition for the defaults section is clear and follows best practices with proper type annotations.
1982-2000: Strong security defaults for pod security context.The security context configuration follows security best practices:
- Uses non-root user (65532)
- Enables runAsNonRoot
- Sets appropriate fsGroup
- Uses OnRootMismatch for fsGroupChangePolicy
2275-2276:❓ Verification inconclusive
Verify in-memory mode resource requirements.
The NGT agent is configured to use in-memory mode. Verify that the pod's memory limits are sufficient for the expected dataset size.
🏁 Script executed:
#!/bin/bash # Check memory settings for in-memory mode rg -A 10 "enable_in_memory_mode:|memory:"Length of output: 65650
Attention: Confirm In-Memory Mode Memory Requirements
The NGT agent is currently enabled in in-memory mode (see charts/vald/values.yaml, around line 2275). However, the automated grep results show many memory-related configurations across the repository, and the output did not directly clarify which settings apply to the NGT pod’s resource limits. Please manually verify that the pod’s memory requests and limits are set high enough to support your expected dataset size. For example, confirm that the memory allocated in the pod spec (as configured in charts/vald/values.yaml and any related resource definitions) aligns with the operational requirements for in-memory mode.
2252-2266:❓ Verification inconclusive
Verify vector dimension size impact on memory usage.
The NGT configuration uses a high dimension size (4096). Ensure this aligns with your memory resource limits as high-dimensional vectors can significantly impact memory usage.
🏁 Script executed:
#!/bin/bash # Check memory settings across agent deployments rg -A 5 "dimension:|memory:"Length of output: 124292
Reassess Memory Footprint for 4096-Dimensional Vectors
The configuration in
charts/vald/values.yamluses a vector dimension of 4096, which is notably higher than sample configurations (often around 100–784). Please verify that this intentional setting fits within your memory resource limits. In high-dimensional scenarios, each vector consumes considerably more memory—so ensure that your deployment’s memory requests/limits (and overall capacity planning) are updated accordingly. Cross-check with your load testing and production benchmarks to confirm that the high dimension size will not lead to unexpected memory pressure.
662-671:❓ Verification inconclusive
Secure the TLS certificate paths.
The TLS certificate paths are using placeholder values (/path/to/cert). Ensure these paths are properly configured in production:
- cert: /path/to/cert
- key: /path/to/key
- ca: /path/to/ca
🏁 Script executed:
#!/bin/bash # Check for TLS path configurations rg -A 3 "cert:|key:|ca:"Length of output: 66574
Secure TLS Certificate Paths in Production
The current configuration in
charts/vald/values.yaml(lines 662–671) still uses placeholder values (/path/to/cert,/path/to/key,/path/to/ca). Please update these to production-appropriate paths or reference environment-specific variables. Note that a global search across the repository shows similar placeholder values in multiple charts, so ensure that all TLS configurations intended for production deployments are properly secured.
- Verify and update the TLS certificate settings in
charts/vald/values.yaml(lines 662–671).- Double-check other similar occurrences (e.g., in related charts) to ensure consistency across production configurations.
charts/vald/schemas/mirror-target-values.yaml (1)
17-28: Well-structured schema definitions with appropriate constraints!The schema annotations provide clear validation rules for the configuration:
- Proper type constraints for all fields
- Meaningful minimum length for host
- Valid port range (0-65535)
.github/workflows/dockers-dev-container-image.yaml (1)
42-52: Review security implications of usingpull_request_target.The
pull_request_targetevent runs with repository secrets and could pose security risks if not properly configured, especially when building Docker images from external PRs.Consider if
pull_requestevent with appropriate permissions is sufficient for your use case..github/workflows/dockers-binfmt-image.yaml (1)
17-18:✅ Verification successful
Important: These workflow files are generated.
As indicated by the comment, these files are generated by
hack/docker/gen/main.go. Any changes should be made to the generator rather than modifying these files directly.To make changes, update the generator at:
🏁 Script executed:
#!/bin/bash # Verify the generator exists and contains the relevant configuration cat hack/docker/gen/main.goLength of output: 38596
IMPORTANT: This workflow file is auto-generated – do not modify it directly.
The file clearly states it is generated by the script at
hack/docker/gen/main.go. To make any changes, update the generator rather than editing this file manually.
- Verified that
hack/docker/gen/main.goexists and includes the directive.- Any modifications must be made in the generator to ensure all derived files remain consistent.
.github/workflows/dockers-buildbase-image.yaml (1)
46-58:✅ Verification successful
Review security implications of
pull_request_target.The workflow uses
pull_request_targetwhich runs with repository secrets. Ensure that the reused workflow (_docker-image.yaml) has proper security checks before accessing sensitive data.Also applies to: 63-65
🏁 Script executed:
#!/bin/bash # Check if the reused workflow has security measures rg -A 5 "pull_request_target" .github/workflows/_docker-image.yamlLength of output: 408
Security Checks Verified for
pull_request_targetUsageOur investigation confirms that the reused workflow (
.github/workflows/_docker-image.yaml) enforces appropriate security measures. Specifically, it restrictspull_request_targetexecution to only occur when the pull request is labeled withci/approved(in addition to push events to trusted branches). This adequately limits the exposure of repository secrets as noted. No further action regarding this security concern is required..github/workflows/dockers-loadtest-image.yaml (1)
17-18: LGTM! Important notice about file generation.The comment clearly indicates that this file is generated and should not be edited directly.
.github/workflows/dockers-index-operator-image.yaml (1)
239-244: LGTM! Well-structured workflow configuration.The workflow is correctly configured to use the reusable workflow with appropriate parameters and secret inheritance.
.github/workflows/dockers-readreplica-rotate-image.yaml (1)
238-242: LGTM! Well-structured workflow configuration.The workflow is correctly configured to use the reusable workflow with appropriate parameters and secret inheritance.
.github/workflows/dockers-index-save-image.yaml (1)
242-246: LGTM! Well-structured workflow configuration.The workflow is correctly configured to use the reusable workflow with appropriate parameters and secret inheritance.
.github/workflows/dockers-index-deletion-image.yaml (2)
23-25:❓ Verification inconclusive
Verify branch pattern exclusions.
The branch pattern
!release/v*.*.*might not catch all patch version branches if they follow a different format.
🏁 Script executed:
#!/bin/bash # Check for any release branches that might not be caught by the exclusion git branch -r | grep -E 'release/v[0-9]+\.[0-9]+\.[0-9]+'Length of output: 59
Action Required: Verify Branch Pattern Exclusions
It appears our initial script did not return any output, leaving uncertainty about whether all patch version branches are appropriately excluded. Please manually review the available remote branches to confirm that branches such as those with unexpected formatting (e.g., qualifiers like pre-release tags) aren’t slipping through. For example, you might run:
git branch -r | grep -E '^ *origin/release/v'and inspect the resulting branch names to ensure the pattern
!release/v*.*.*is sufficient.
31-31:✅ Verification successful
Review security implications of
pull_request_targetevent.The workflow uses both
pull_requestandpull_request_targetevents with identical path filters. While this allows the workflow to access repository secrets when triggered by external pull requests, it could pose security risks if not properly configured, aspull_request_targetruns in the context of the base repository.To ensure secure usage of
pull_request_target, verify that:
- The workflow only accesses secrets during the build step
- The workflow doesn't execute untrusted code from the PR
Also applies to: 136-142
🏁 Script executed:
#!/bin/bash # Check if any secrets are used outside the build job rg -A 5 'secrets:|env:' .github/workflows/Length of output: 40666
Security Brief: Controlled Secret Access Confirmed
Our verification indicates that the current configuration in
.github/workflows/dockers-index-deletion-image.yamlshows only thepull_requesttrigger (as evidenced by the diff snippet) and no instance of apull_request_targetevent. Additionally, our search output confirms that secrets (viasecrets: inherit) are used in a manner consistent with limiting access to the build steps.Please ensure that:
- All secret accesses remain confined to the build job.
- No steps inadvertently execute untrusted code from pull request sources.
If any further modifications reintroduce the
pull_request_targetevent or change the secret handling, please revalidate these aspects..github/workflows/dockers-index-creation-image.yaml (1)
242-246: LGTM! Consistent job configuration.The job configuration correctly uses the reusable workflow and passes the appropriate target.
.github/workflows/dockers-gateway-lb-image.yaml (1)
255-261: LGTM! Consistent secrets handling.The workflow correctly inherits secrets from the caller context.
.github/workflows/dockers-benchmark-operator-image.yaml (1)
142-258:⚠️ Potential issueSame security concern with
pull_request_targetevent.The workflow has the same security vulnerability with
pull_request_targetas the gateway-filter workflow..github/workflows/dockers-benchmark-job-image.yaml (1)
145-258:⚠️ Potential issueSame security concern with
pull_request_targetevent.The workflow has the same security vulnerability with
pull_request_targetas the gateway-filter workflow..github/workflows/dockers-gateway-mirror-image.yaml (1)
145-258:⚠️ Potential issueSame security concern with
pull_request_targetevent.The workflow has the same security vulnerability with
pull_request_targetas the gateway-filter workflow..github/workflows/dockers-manager-index-image.yaml (1)
146-260:⚠️ Potential issueSame security concern with
pull_request_targetevent.The workflow has the same security vulnerability with
pull_request_targetas the gateway-filter workflow..github/workflows/dockers-agent-faiss-image.yaml (2)
264-268: LGTM! Reusable workflow implementation.The job configuration correctly uses the reusable workflow pattern with proper target specification and secret inheritance.
31-32:✅ Verification successful
Security: Review pull_request_target event usage.
The workflow uses
pull_request_targetwhich runs with repository secrets in the context of the base repository. This could be a security risk if not properly configured, as it could expose sensitive information to malicious PRs.Ensure that the workflow is not checking out or running untrusted code from the PR. The current configuration looks safe as it's using a reusable workflow, but verify that
_docker-image.yamlfollows security best practices:Also applies to: 147-148
🏁 Script executed:
#!/bin/bash # Description: Check if _docker-image.yaml has proper security measures rg -A 5 "uses: actions/checkout@" .github/workflows/_docker-image.yamlLength of output: 686
Security validation passed for pull_request_target usage.
- Verified that the reusable workflow (
_docker-image.yaml) employsactions/checkout@v4with proper security measures.- Confirmed that the checkout and related steps follow best practices, mitigating risks associated with running untrusted PR code.
- Similar security measures have been observed at the relevant sections (including lines 147-148).
No further modifications are required.
.github/workflows/dockers-agent-ngt-image.yaml (1)
1-273: LGTM! Consistent workflow implementation.The workflow follows the same secure pattern as agent-faiss, with appropriate triggers and path filters.
.github/workflows/dockers-agent-sidecar-image.yaml (1)
1-299: LGTM! Consistent workflow implementation.The workflow follows the same secure pattern as other agent workflows, with appropriate triggers and path filters.
charts/vald-benchmark-operator/values.yaml (1)
717-732: LGTM! Proper use of Kubernetes downward API.The environment variables are correctly configured to access pod and node metadata using the Kubernetes downward API.
charts/vald-benchmark-operator/schemas/job-values.yaml (2)
17-25: LGTM! Well-defined target schema.The target schema is properly defined with:
- Required fields validation
- Port range validation (0-65535)
- Host string validation
47-49: LGTM! Dataset URL field addition.The dataset URL field is properly documented and allows for custom HDF5 file usage.
.github/workflows/dockers-ci-container-image.yaml (3)
17-18: LGTM! Clear generation notice.The file is properly marked as generated with a reference to the source generator script.
20-52: LGTM! Well-structured event triggers.The workflow triggers are comprehensive and well-defined:
- Push events target main and release branches with proper version tag patterns
- Path filters for pull requests include all relevant files for the ci-container image
53-59: LGTM! Clean job configuration.The job configuration is properly structured:
- Uses reusable workflow for consistency
- Platform is correctly specified as linux/amd64
- Secrets are properly inherited
.github/workflows/dockers-buildkit-image.yaml (3)
17-18: LGTM! Clear generation notice.The file is properly marked as generated with a reference to the source generator script.
20-58: LGTM! Well-structured event triggers.The workflow triggers are comprehensive and well-defined:
- Hourly schedule for regular builds
- Push events target main and release branches with proper version tag patterns
- Path filters properly exclude test files and include relevant files
59-65: LGTM! Clean job configuration.The job configuration is properly structured:
- Uses reusable workflow for consistency
- Platforms include both linux/amd64 and linux/arm64
- Secrets are properly inherited
kmrmt
requested review from
a team,
kpango and
datelier
and removed request for
a team
Deploying vald with
|
| Latest commit: |
388de1f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://260dc019.vald.pages.dev |
| Branch Preview URL: | https://bugfix-yaml-enable-retain-li.vald.pages.dev |
* enable retain_line_breaks * add line breaks * format --------- Co-authored-by: Kiichiro YUKAWA <[email protected]>
* enable retain_line_breaks * add line breaks * format --------- Co-authored-by: Kosuke Morimoto <[email protected]> Co-authored-by: Kiichiro YUKAWA <[email protected]>
Description
SSIA
This pull request resolves same issue for yaml formatting trouble.
Merge only this or #2849.
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
Chores