-
Notifications
You must be signed in to change notification settings - Fork 1k
chore: add script to update argocd dependencies and images #1877
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
base: master
Are you sure you want to change the base?
Conversation
37bc3ba to
82325f7
Compare
770c447 to
7842460
Compare
7842460 to
b4fa01d
Compare
WalkthroughThis PR introduces a comprehensive dependency update automation system for the operator. It adds a GitHub Actions workflow job to verify dependencies remain current, a Makefile target to execute updates, and a Go-based script that upgrades Argo CD artifacts including container images, CRDs, Go modules, manifests, and test references. Changes
Sequence DiagramsequenceDiagram
actor GHA as GitHub Actions
participant WF as Workflow Job
participant Make as make target
participant Script as Go Script
participant ArgoCDRepo as Argo CD Repo
participant OperatorRepo as Operator Repo Files
GHA->>WF: Trigger verify_argo_cd_has_updated_dependencies
WF->>Make: Execute make update-dependencies
Make->>Script: Execute hack/update-dependencies-script/run.sh
Script->>Script: Read ARGO_CD_TARGET_VERSION from Makefile
Script->>ArgoCDRepo: Clone Argo CD at target version
ArgoCDRepo-->>Script: Cloned repository
Script->>OperatorRepo: Update container images in common/defaults.go
Script->>OperatorRepo: Copy CRD YAMLs from cloned Argo CD
Script->>OperatorRepo: Update go.mod and sync versions
Script->>OperatorRepo: Regenerate manifests and bundle
Script->>OperatorRepo: Update Dockerfile image reference
Script->>OperatorRepo: Update dex_test.go image reference
Script-->>Make: Return control
Make-->>WF: Script completes
WF->>WF: Verify no unexpected diffs (ignore createdAt)
WF-->>GHA: Job result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (6)
config/crd/bases/argoproj.io_applications.yaml (1)
5890-5890: Formatting-only change LGTM; consider explicit status subresource for consistency.No functional change detected (trailing newline;
subresources: {}unchanged). If aligning with ApplicationSet CRD and controller behavior, you may want to explicitly enable the status subresource here as well.Optional tweak:
- subresources: {} + subresources: + status: {}Please confirm whether upstream Argo CD’s Application CRD currently declares
statusas a subresource; if it does, mirroring it here avoids mixed spec/status updates and eases RBAC separation.hack/update-dependencies-script/go.mod (2)
1-3: Consider aligning Go version with the main project.The script uses
go 1.20, while the main project's go.mod specifiesgo 1.24.6. This version mismatch could lead to compatibility issues or inconsistent behavior.Apply this diff to align the Go version:
module github.com/argoproj-labs/argocd-operator/dependency-upgrade -go 1.20 +go 1.24.6
6-6: Consider using a more recent go-github version.The script uses
github.com/google/go-github/v58, while the main project uses v69 and v72. Although this may work fine for the script's needs, using a more aligned version could prevent potential API incompatibilities.hack/update-dependencies-script/README.md (1)
10-12: Add language identifiers to fenced code blocks.Markdown best practice is to specify the language for syntax highlighting.
Apply this diff:
-``` +```makefile ARGO_CD_TARGET_VERSION ?= 3.1.8Then run the script:
-+bash
make update-dependenciesAlso applies to: 15-17
Makefile (1)
373-377: Consider quoting the script path.The script path should be quoted to handle edge cases with spaces or special characters in paths.
Apply this diff:
.PHONY: update-dependencies update-dependencies: - "hack/update-dependencies-script/run.sh" + ./hack/update-dependencies-script/run.shAlternatively, if you prefer the quoted form, ensure it's properly quoted:
- "hack/update-dependencies-script/run.sh" + ./hack/update-dependencies-script/run.shNote: The
./prefix is conventional in Makefiles for relative script paths and doesn't require quotes. If you keep the quoted form, it works but the./prefix is more idiomatic.hack/update-dependencies-script/utils.go (1)
53-63: Remove unreachable return statement.After
exitWithErrorcallsos.Exit(1), the return on line 58 is never reached.Apply this diff:
func stripImagePrefix(line string) string { line = strings.TrimSpace(line) if !strings.HasPrefix(line, "image:") { exitWithError(fmt.Errorf("unexpected image format on line: %s", line)) - return "" } return strings.TrimPrefix(line, "image: ") - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumhack/update-dependencies-script/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
.github/workflows/lint.yaml(2 hunks)Makefile(2 hunks)build/util/Dockerfile(1 hunks)common/defaults.go(2 hunks)config/crd/bases/argoproj.io_applications.yaml(1 hunks)config/crd/bases/argoproj.io_applicationsets.yaml(1 hunks)config/crd/bases/argoproj.io_appprojects.yaml(1 hunks)controllers/argocd/applicationset_test.go(2 hunks)controllers/argocd/dex_test.go(1 hunks)go.mod(2 hunks)hack/update-dependencies-script/README.md(1 hunks)hack/update-dependencies-script/go.mod(1 hunks)hack/update-dependencies-script/main.go(1 hunks)hack/update-dependencies-script/run.sh(1 hunks)hack/update-dependencies-script/utils.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
controllers/argocd/applicationset_test.go (1)
common/defaults.go (1)
ArgoCDDefaultArgoVersion(73-73)
🪛 checkmake (0.2.2)
Makefile
[warning] 375-375: Missing required phony target "all"
(minphony)
[warning] 375-375: Missing required phony target "clean"
(minphony)
[warning] 375-375: Missing required phony target "test"
(minphony)
🪛 markdownlint-cli2 (0.18.1)
hack/update-dependencies-script/README.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
hack/update-dependencies-script/run.sh
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: build-operator
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: Verify Argo CD has updated dependencies, for the given target version
🔇 Additional comments (11)
go.mod (2)
6-6: LGTM! Argo CD dependency updated to v3.1.9.The version bump aligns with the PR objectives to upgrade Argo CD dependencies.
178-178: Good documentation practice.The comment clearly indicates the origin of the replace block, making it easier to trace and maintain.
config/crd/bases/argoproj.io_appprojects.yaml (1)
366-366: LGTM! Formatting adjustment only.This change appears to be an end-of-file newline adjustment with no semantic impact.
controllers/argocd/dex_test.go (1)
586-586: Good documentation practice.The inline comment clearly indicates that this image value is managed by the dependency update script, helping maintainers understand the maintenance workflow.
config/crd/bases/argoproj.io_applicationsets.yaml (1)
17724-17724: Enable status subresource for ApplicationSet CRD.The addition of the empty
status: {}subresource declaration enables the status subresource for the ApplicationSet CRD, which is a standard Kubernetes feature allowing the status field to be updated independently from the spec. This change aligns with the ArgoCD v3.1.9 dependency upgrade.Please confirm that this change is an intentional part of the ArgoCD v3.1.9 upgrade and that similar updates have been applied to other CRDs as needed.
common/defaults.go (1)
73-73: LGTM! Version constants updated correctly.The Argo CD and Redis image digests have been updated appropriately with inline version comments for traceability.
Also applies to: 194-194, 197-197
build/util/Dockerfile (1)
1-2: LGTM! Dockerfile base image updated correctly.The Argo CD base image has been updated to v3.1.9 with the correct digest, consistent with the version constant in common/defaults.go.
.github/workflows/lint.yaml (1)
1-1: LGTM! CI validation for dependency updates is well-designed.The new verification job ensures that dependency updates remain consistent with the target version by running the update script and checking for diffs. The
--ignore-matching-linesforcreatedAtis appropriate for handling generated timestamps.Also applies to: 32-43
controllers/argocd/applicationset_test.go (1)
454-454: LGTM! Tests now use centralized version constant.Refactoring the tests to use
common.ArgoCDDefaultArgoVersioninstead of hardcoded digests improves maintainability and ensures tests automatically reflect version updates.Also applies to: 1484-1484, 1505-1505
Makefile (1)
8-13: LGTM! Well-documented version tracking variable.The
ARGO_CD_TARGET_VERSIONvariable is clearly documented with helpful notes about usage and format requirements. This provides a clear entry point for dependency updates.hack/update-dependencies-script/utils.go (1)
81-96: Note: Order is not preserved during deduplication.The function uses a map for deduplication, which means the output order is non-deterministic due to Go's map iteration randomization. If order preservation is required, consider using a slice-based approach with a map for tracking seen elements.
If order preservation is important for your use case, consider this alternative:
func removeDuplicateLines(in []string) []string { seen := make(map[string]bool) var res []string for _, inVal := range in { if !seen[inVal] { seen[inVal] = true res = append(res, inVal) } } return res }
| replaceBlockFromArgoCDLines, replaceBlockStart, replaceBlockEnd := readReplaceBlockFromGoMod(argoCDRepoRootGoModPath) | ||
| replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd] | ||
|
|
||
| argocdOperatorGoModPath := filepath.Join(argocdOperatorRoot, "go.mod") | ||
|
|
||
| argocdOperatorLines, argocdOperatorReplaceStart, argocdOperatorReplaceEnd := readReplaceBlockFromGoMod(argocdOperatorGoModPath) | ||
|
|
||
| newArgoCDOperatorGoModFileContents := "" | ||
|
|
||
| for x, line := range argocdOperatorLines { | ||
| if x == argocdOperatorReplaceStart { | ||
|
|
||
| newArgoCDOperatorGoModFileContents += "\t// This replace block is from Argo CD " + targetArgoCDVersion + " go.mod\n" | ||
|
|
||
| // inject the replace block from argocd's go.mod | ||
| for _, replaceBlockFromArgoCDLine := range replaceBlockFromArgoCD { | ||
| newArgoCDOperatorGoModFileContents += replaceBlockFromArgoCDLine + "\n" | ||
| } | ||
|
|
||
| } else if x > argocdOperatorReplaceStart && x < argocdOperatorReplaceEnd { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy the entire replace block (do not drop the last line)
readReplaceBlockFromGoMod returns indices inclusive of the block, but slicing with replaceBlockEnd as the upper bound excludes the final replace entry. The generated go.mod therefore loses the last replace line. Include replaceBlockEnd+1 in the slice.
- replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd]
+ replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart : replaceBlockEnd+1]📝 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.
| replaceBlockFromArgoCDLines, replaceBlockStart, replaceBlockEnd := readReplaceBlockFromGoMod(argoCDRepoRootGoModPath) | |
| replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd] | |
| argocdOperatorGoModPath := filepath.Join(argocdOperatorRoot, "go.mod") | |
| argocdOperatorLines, argocdOperatorReplaceStart, argocdOperatorReplaceEnd := readReplaceBlockFromGoMod(argocdOperatorGoModPath) | |
| newArgoCDOperatorGoModFileContents := "" | |
| for x, line := range argocdOperatorLines { | |
| if x == argocdOperatorReplaceStart { | |
| newArgoCDOperatorGoModFileContents += "\t// This replace block is from Argo CD " + targetArgoCDVersion + " go.mod\n" | |
| // inject the replace block from argocd's go.mod | |
| for _, replaceBlockFromArgoCDLine := range replaceBlockFromArgoCD { | |
| newArgoCDOperatorGoModFileContents += replaceBlockFromArgoCDLine + "\n" | |
| } | |
| } else if x > argocdOperatorReplaceStart && x < argocdOperatorReplaceEnd { | |
| replaceBlockFromArgoCDLines, replaceBlockStart, replaceBlockEnd := readReplaceBlockFromGoMod(argoCDRepoRootGoModPath) | |
| replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart : replaceBlockEnd+1] | |
| argocdOperatorGoModPath := filepath.Join(argocdOperatorRoot, "go.mod") | |
| argocdOperatorLines, argocdOperatorReplaceStart, argocdOperatorReplaceEnd := readReplaceBlockFromGoMod(argocdOperatorGoModPath) | |
| newArgoCDOperatorGoModFileContents := "" | |
| for x, line := range argocdOperatorLines { | |
| if x == argocdOperatorReplaceStart { | |
| newArgoCDOperatorGoModFileContents += "\t// This replace block is from Argo CD " + targetArgoCDVersion + " go.mod\n" | |
| // inject the replace block from argocd's go.mod | |
| for _, replaceBlockFromArgoCDLine := range replaceBlockFromArgoCD { | |
| newArgoCDOperatorGoModFileContents += replaceBlockFromArgoCDLine + "\n" | |
| } | |
| } else if x > argocdOperatorReplaceStart && x < argocdOperatorReplaceEnd { |
🤖 Prompt for AI Agents
In hack/update-dependencies-script/main.go around lines 379 to 398 the code
slices replaceBlockFromArgoCDLines using
replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd], which omits the
final line because readReplaceBlockFromGoMod returns inclusive end indices;
change the slice to use
replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd+1] so the entire
replace block (including the last replace entry) is copied into the generated
go.mod.
| // Extract Digest value from JSON | ||
| var jsonMap map[string]any | ||
| if err := json.Unmarshal([]byte(stdout), &jsonMap); err != nil { | ||
| exitWithError(fmt.Errorf("unexpected unmarshal error: %v", err)) | ||
| return nil | ||
| } | ||
| digestVal := jsonMap["Digest"] | ||
|
|
||
| if digestVal == "" { | ||
| exitWithError(fmt.Errorf("unable to extract digest val for %s", url)) | ||
| return nil | ||
| } | ||
|
|
||
| return &processedContainerImage{ | ||
| version: url[strings.Index(url, ":")+1:], | ||
| sha256Digest: digestVal.(string), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defensively assert the Digest field before type asserting
If skopeo ever omits the Digest key (e.g., format change or registry quirk), digestVal.(string) will panic. Guard both presence and type so the script exits cleanly.
- digestVal := jsonMap["Digest"]
-
- if digestVal == "" {
+ digestVal, ok := jsonMap["Digest"].(string)
+ if !ok || digestVal == "" {
exitWithError(fmt.Errorf("unable to extract digest val for %s", url))
return nil
}
return &processedContainerImage{
version: url[strings.Index(url, ":")+1:],
- sha256Digest: digestVal.(string),
+ sha256Digest: digestVal,
}🤖 Prompt for AI Agents
In hack/update-dependencies-script/main.go around lines 526 to 542, the code
unsafely type-asserts jsonMap["Digest"] to string which can panic if the key is
missing or not a string; update the code to first check that the key exists and
that the value is a string (e.g., v, ok := jsonMap["Digest"]; if !ok || v == nil
{ exitWithError(...) } s, ok := v.(string); if !ok || s == "" {
exitWithError(...) }) and then use the validated string for sha256Digest so the
script exits cleanly rather than panicking.
|
|
||
| SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
|
|
||
| cd $SCRIPT_DIR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for directory change.
The cd command should handle failures to prevent unexpected behavior if the directory doesn't exist or is inaccessible.
Apply this diff:
-cd $SCRIPT_DIR
+cd "$SCRIPT_DIR" || exit 1Note: Also added quotes around $SCRIPT_DIR to handle paths with spaces.
📝 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.
| cd $SCRIPT_DIR | |
| cd "$SCRIPT_DIR" || exit 1 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In hack/update-dependencies-script/run.sh around line 5, the script does an
unguarded cd $SCRIPT_DIR which can fail and continue execution; change it to cd
"$SCRIPT_DIR" and check its exit status, printing an explanatory error to stderr
(or using echo) and exiting non‑zero (e.g., exit 1) if the cd fails so the
script stops instead of running in the wrong directory.
| func copyFile(srcParam, dstParam string) error { | ||
| srcFile, err := os.Open(srcParam) | ||
| if err != nil { | ||
| return fmt.Errorf("could not open source file %s: %w", srcParam, err) | ||
| } | ||
|
|
||
| destFile, err := os.Create(dstParam) | ||
| if err != nil { | ||
| return fmt.Errorf("could not create destination file %s: %w", dstParam, err) | ||
| } | ||
|
|
||
| _, err = io.Copy(destFile, srcFile) | ||
| if err != nil { | ||
| return fmt.Errorf("could not copy content from %s to %s: %w", srcParam, dstParam, err) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add explicit file closing with defer.
The files should be explicitly closed using defer to prevent resource leaks, especially if errors occur after opening.
Apply this diff:
func copyFile(srcParam, dstParam string) error {
srcFile, err := os.Open(srcParam)
if err != nil {
return fmt.Errorf("could not open source file %s: %w", srcParam, err)
}
+ defer srcFile.Close()
destFile, err := os.Create(dstParam)
if err != nil {
return fmt.Errorf("could not create destination file %s: %w", dstParam, err)
}
+ defer destFile.Close()
_, err = io.Copy(destFile, srcFile)
if err != nil {
return fmt.Errorf("could not copy content from %s to %s: %w", srcParam, dstParam, err)
}
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func copyFile(srcParam, dstParam string) error { | |
| srcFile, err := os.Open(srcParam) | |
| if err != nil { | |
| return fmt.Errorf("could not open source file %s: %w", srcParam, err) | |
| } | |
| destFile, err := os.Create(dstParam) | |
| if err != nil { | |
| return fmt.Errorf("could not create destination file %s: %w", dstParam, err) | |
| } | |
| _, err = io.Copy(destFile, srcFile) | |
| if err != nil { | |
| return fmt.Errorf("could not copy content from %s to %s: %w", srcParam, dstParam, err) | |
| } | |
| return nil | |
| } | |
| func copyFile(srcParam, dstParam string) error { | |
| srcFile, err := os.Open(srcParam) | |
| if err != nil { | |
| return fmt.Errorf("could not open source file %s: %w", srcParam, err) | |
| } | |
| defer srcFile.Close() | |
| destFile, err := os.Create(dstParam) | |
| if err != nil { | |
| return fmt.Errorf("could not create destination file %s: %w", dstParam, err) | |
| } | |
| defer destFile.Close() | |
| _, err = io.Copy(destFile, srcFile) | |
| if err != nil { | |
| return fmt.Errorf("could not copy content from %s to %s: %w", srcParam, dstParam, err) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In hack/update-dependencies-script/utils.go around lines 98 to 115, the file
handles opened with os.Open and os.Create are not closed, risking resource
leaks; add defer calls to close both srcFile and destFile immediately after each
successful open/create, and capture/propagate any Close error from destFile
(wrap it with fmt.Errorf) so a failed Close doesn't get ignored (use a named
return error or check the copy error and then check destFile.Close() and return
the first non-nil error).
| func getFileContentsAsLines(filePath string) []string { | ||
| bytes, err := os.ReadFile(filePath) | ||
| if err != nil { | ||
| exitWithError(fmt.Errorf("unable to dex_test.go: %v", err)) | ||
| return nil | ||
| } | ||
|
|
||
| return strings.Split(string(bytes), "\n") | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error message and remove unreachable code.
The error message incorrectly mentions "dex_test.go", and line 137 is unreachable after exitWithError.
Apply this diff:
func getFileContentsAsLines(filePath string) []string {
bytes, err := os.ReadFile(filePath)
if err != nil {
- exitWithError(fmt.Errorf("unable to dex_test.go: %v", err))
- return nil
+ exitWithError(fmt.Errorf("unable to read file %s: %v", filePath, err))
}
return strings.Split(string(bytes), "\n")
-
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getFileContentsAsLines(filePath string) []string { | |
| bytes, err := os.ReadFile(filePath) | |
| if err != nil { | |
| exitWithError(fmt.Errorf("unable to dex_test.go: %v", err)) | |
| return nil | |
| } | |
| return strings.Split(string(bytes), "\n") | |
| } | |
| func getFileContentsAsLines(filePath string) []string { | |
| bytes, err := os.ReadFile(filePath) | |
| if err != nil { | |
| exitWithError(fmt.Errorf("unable to read file %s: %v", filePath, err)) | |
| } | |
| return strings.Split(string(bytes), "\n") | |
| } |
🤖 Prompt for AI Agents
In hack/update-dependencies-script/utils.go around lines 133 to 142, the error
message incorrectly references "dex_test.go" and the return after exitWithError
is unreachable; change the fmt.Errorf message to include the actual filePath
(e.g., fmt.Errorf("unable to read %s: %v", filePath, err)) and remove the
unreachable "return nil" after calling exitWithError so the function either
exits or returns only once; ensure the function still returns
strings.Split(string(bytes), "\n") on success.
b4fa01d to
fc41142
Compare
There was a problem hiding this 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
♻️ Duplicate comments (1)
hack/update-dependencies-script/run.sh (1)
5-5: Add error handling for directory change (flagged in previous review).Line 5 contains an unguarded
cdwithout error handling. This issue was previously flagged and should be fixed before merge.Apply this diff:
-cd $SCRIPT_DIR +cd "$SCRIPT_DIR" || exit 1This ensures the script exits if the directory change fails (e.g., directory doesn't exist or lacks permissions) and handles paths containing spaces.
🧹 Nitpick comments (1)
hack/update-dependencies-script/README.md (1)
10-12: Add language specifiers to fenced code blocks for proper syntax highlighting.The code blocks are missing language identifiers, which helps with readability and enables syntax highlighting.
Apply this diff:
-Example: -``` +Example: +```makefile ARGO_CD_TARGET_VERSION ?= 3.1.8 -``` +``` -Then run the script: -``` +Then run the script: +```bash make update-dependencies -``` +```Also applies to: 15-17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
hack/update-dependencies-script/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.github/workflows/ci-build.yaml(5 hunks).github/workflows/lint.yaml(2 hunks)Makefile(2 hunks)hack/update-dependencies-script/README.md(1 hunks)hack/update-dependencies-script/go.mod(1 hunks)hack/update-dependencies-script/main.go(1 hunks)hack/update-dependencies-script/run.sh(1 hunks)hack/update-dependencies-script/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- hack/update-dependencies-script/go.mod
- .github/workflows/lint.yaml
- hack/update-dependencies-script/utils.go
- hack/update-dependencies-script/main.go
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 375-375: Missing required phony target "all"
(minphony)
[warning] 375-375: Missing required phony target "clean"
(minphony)
[warning] 375-375: Missing required phony target "test"
(minphony)
🪛 markdownlint-cli2 (0.18.1)
hack/update-dependencies-script/README.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
hack/update-dependencies-script/run.sh
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-operator
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: Verify Argo CD has updated dependencies, for the given target version
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/ci-build.yaml (1)
105-106: Verify the background script doesn't introduce test timing or resource conflicts.The new
hack/df-loop.sh &runs in the background throughout test execution without explicit synchronization or completion verification. Ensure this background process:
- Doesn't conflict with test resources or namespaces
- Properly terminates after tests complete
- Doesn't mask test failures if it exits early
Can you clarify the purpose of
hack/df-loop.shand verify that running it concurrently with the test suite doesn't introduce flakiness or resource contention? If this script should run until tests complete, consider replacing the bare&with explicit cleanup (e.g.,trapto kill the background process on exit).
| # Updates upstream dependencies throughout the repository | ||
| .PHONY: update-dependencies | ||
| update-dependencies: | ||
| "hack/update-dependencies-script/run.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove quotes from script path in the make recipe.
Line 377 has the script path quoted, which causes the shell to interpret it as a literal filename (with quotes included) rather than executing the script. Makefile recipes are passed to the shell, so shell quoting rules apply.
Apply this diff:
# Updates upstream dependencies throughout the repository
.PHONY: update-dependencies
update-dependencies:
- "hack/update-dependencies-script/run.sh"
+ hack/update-dependencies-script/run.shAlternatively, if the script path needs quoting for safety (e.g., paths with spaces), use the shell's command expansion syntax:
# Updates upstream dependencies throughout the repository
.PHONY: update-dependencies
update-dependencies:
+ $(shell pwd)/hack/update-dependencies-script/run.shHowever, the simpler unquoted path is preferable for this use case.
📝 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.
| # Updates upstream dependencies throughout the repository | |
| .PHONY: update-dependencies | |
| update-dependencies: | |
| "hack/update-dependencies-script/run.sh" | |
| # Updates upstream dependencies throughout the repository | |
| .PHONY: update-dependencies | |
| update-dependencies: | |
| hack/update-dependencies-script/run.sh |
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 375-375: Missing required phony target "all"
(minphony)
[warning] 375-375: Missing required phony target "clean"
(minphony)
[warning] 375-375: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
In Makefile around lines 374 to 377, the update-dependencies recipe uses a
quoted script path which causes the shell to treat the quotes as part of the
filename; remove the surrounding double quotes so the shell executes
hack/update-dependencies-script/run.sh directly, or if you must protect spaces
use the shell command invocation form instead (e.g., use shell expansion) so the
script is executed rather than treated as a literal quoted filename.
There was a problem hiding this 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
Plan: Pro
📒 Files selected for processing (1)
hack/df-loop.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: build
- GitHub Check: build-operator
- GitHub Check: Run end-to-end tests (v1.27.1)
There was a problem hiding this 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
♻️ Duplicate comments (2)
Makefile (1)
374-377: Remove quotes from the script path in the Makefile recipe.The script path is wrapped in double quotes, which causes the shell to treat the quotes as part of the literal filename rather than executing the script. Makefile recipes are passed directly to the shell—shell quoting rules apply—but the quotes here will cause a "command not found" error at runtime.
Apply this diff to fix the issue:
# Updates upstream dependencies throughout the repository .PHONY: update-dependencies update-dependencies: - "hack/update-dependencies-script/run.sh" + hack/update-dependencies-script/run.shhack/df-loop.sh (1)
1-10: Add signal handling for graceful CI process termination (previously noted, still unaddressed).The script lacks signal handlers for
SIGTERMandSIGINT, preventing clean exit when the CI workflow terminates it. Since this runs as a background process, orphaned loops or delayed test completion are likely.Apply this diff to add signal handling:
#!/bin/bash -# Loop forever, running df -h and sleeping for 5 seconds +# Loop forever, running df -h and sleeping for 5 seconds +# Monitors disk and namespace state during CI test execution + +# Clean exit on termination signals +trap 'exit 0' SIGTERM SIGINT + while true; do echo "--------------------" kubectl get namespaces echo "----" df -h | grep root sleep 10 done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile(4 hunks)hack/df-loop.sh(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 375-375: Missing required phony target "all"
(minphony)
[warning] 375-375: Missing required phony target "clean"
(minphony)
[warning] 375-375: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: build
- GitHub Check: Verify Argo CD has updated dependencies, for the given target version
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: build-operator
🔇 Additional comments (3)
Makefile (3)
8-13: LGTM on ARGO_CD_TARGET_VERSION introduction.Clear documentation, sensible default (3.1.9), and explicit guidance on the expected format. The integration with the
update-dependenciestarget supports the overall dependency management workflow.
15-21: Improved container runtime detection with graceful fallback.Changing from a hard error to a warning when neither Docker nor Podman is found is a pragmatic improvement. It allows the Makefile to remain usable in environments where container operations aren't needed or possible (e.g., CI runners with pre-installed tooling or local development where the user hasn't yet installed a runtime).
349-349: Verify the rationale for reducing Ginkgo parallel concurrency from 4 to 3 processes.The change reduces parallel test concurrency from 4 procs to 3. This may reflect resource constraints or stability improvements in the test environment, but the change warrants verification to ensure it doesn't inadvertently reduce test throughput for CI pipelines or local development.
There was a problem hiding this 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
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-build.yaml(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build
- GitHub Check: build-operator
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: Run golangci-lint and gosec
🔇 Additional comments (3)
.github/workflows/ci-build.yaml (3)
5-7: YAML formatting and quoting improvements.These changes consistently quote string literals in YAML, which is a best practice for clarity and robustness. No functional changes here.
Also applies to: 12-14, 33-33, 80-80
49-49: Matrix and comment formatting adjustments.Minor formatting adjustments for consistency and clarity. No functional impact.
Also applies to: 70-72
54-56: KUTTL checksum verified as correct. The SHA256 sumad21c0d2be495a8f6cfc0821e592ae61afd191ebd453156d9b204e2524fceaf2matches the official KUTTL v0.10.0kubectl-kuttl_0.10.0_linux_x86_64binary. Version and checksum are consistent.
|
|
||
| func main() { | ||
|
|
||
| wd, err := os.Getwd() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is correct when called as make>run.sh>main.go, though I would like to propose passing in the repo root as a parameter. That way, we can run go run straight from the Makefile and drop the shell script middleman. I find it more robust because then main.go would not rely on being invoked in a specific way - you test the root is the real root few lines after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/ginkgo/fixture/fixture.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ginkgo/fixture/fixture.go (1)
tests/ginkgo/fixture/os/fixture.go (1)
ExecCommandWithOutputParam(15-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: Verify Argo CD has updated dependencies, for the given target version
- GitHub Check: build-operator
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: build
🔇 Additional comments (2)
tests/ginkgo/fixture/fixture.go (2)
7-7: LGTM!The
strconvimport is correctly added to support integer parsing in the new disk-space check function.
89-89: Good placement for the disk-space check.Invoking the disk-space check at the start of
EnsureParallelCleanSlate()ensures tests don't proceed until sufficient space is available, which aligns well with the test infrastructure goals. However, this assumes the critical issues inwaitForRootPartitionToHaveMinimumDiskSpace()are resolved first.
cd73093 to
40bdac1
Compare
Signed-off-by: Jonathan West <[email protected]>
b0310e1 to
a2db378
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (7)
hack/update-dependencies-script/run.sh (1)
5-5: Add error handling for directory change.The
cdcommand should handle failures to prevent unexpected behavior if the directory doesn't exist. Also, quote the variable to handle paths with spaces.Apply this diff:
-cd $SCRIPT_DIR +cd "$SCRIPT_DIR" || exit 1Makefile (1)
374-377: Remove quotes from script path in the make recipe.The script path is quoted, which causes the shell to interpret it as a literal filename rather than executing the script.
Apply this diff:
# Updates upstream dependencies throughout the repository .PHONY: update-dependencies update-dependencies: - "hack/update-dependencies-script/run.sh" + hack/update-dependencies-script/run.shhack/update-dependencies-script/main.go (5)
121-133: Prevent index overrun when matching dex image block.
lines[idx+1]is accessed without confirming thatidx+1is in bounds. When the loop reaches the final index, this will panic.Apply this diff:
for idx := 0; idx < len(lines); idx++ { + if idx+1 >= len(lines) { + newContent += lines[idx] + "\n" + continue + } if strings.HasPrefix(lines[idx], " Name: \"dex\",") && strings.HasPrefix(lines[idx+1], " Image: \"ghcr.io/dexidp/dex@sha256:") {
164-174: Apply bounds check when updating build/util/Dockerfile.This loop also accesses
lines[idx+1]without verifying bounds. Add a guard before accessing the next line.Apply this diff:
for idx := 0; idx < len(lines); idx++ { + if idx+1 >= len(lines) { + newContent += lines[idx] + "\n" + continue + } // Replace the argocd image references in the Dockerfile with new version if strings.HasPrefix(lines[idx], "# Argo CD v") && strings.HasPrefix(lines[idx+1], "FROM quay.io/argoproj/argocd@sha256") {
379-380: Copy the entire replace block (include the last line).
readReplaceBlockFromGoModreturns indices wherereplaceBlockEndpoints to the last line of content. Slicing with[replaceBlockStart:replaceBlockEnd]excludes the final replace entry. UsereplaceBlockEnd+1to include it.Apply this diff:
- replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart:replaceBlockEnd] + replaceBlockFromArgoCD := replaceBlockFromArgoCDLines[replaceBlockStart : replaceBlockEnd+1]
532-541: Defensively check the Digest field before type assertion.If
skopeoomits theDigestkey or returns a non-string value,digestVal.(string)will panic. Guard both presence and type.Apply this diff:
- digestVal := jsonMap["Digest"] - - if digestVal == "" { + digestVal, ok := jsonMap["Digest"].(string) + if !ok || digestVal == "" { exitWithError(fmt.Errorf("unable to extract digest val for %s", url)) return nil } return &processedContainerImage{ version: url[strings.Index(url, ":")+1:], - sha256Digest: digestVal.(string), + sha256Digest: digestVal, }
225-229: Add missing return statement after exitWithError.Unlike other similar error handling blocks in this file, this block is missing a
returnstatement afterexitWithError. This could lead to a nil pointer dereference on line 229 if the error condition is met.Apply this diff:
targetArgoCDImageLine := removeDuplicateLines(grepForString(installYAMLContents, "quay.io/argoproj/argocd:")) if len(targetArgoCDImageLine) != 1 { exitWithError(fmt.Errorf("unexpected target argo cd image value: %v", targetArgoCDImageLine)) + return nil, nil } targetArgoCDImage := stripImagePrefix(targetArgoCDImageLine[0])
🧹 Nitpick comments (2)
hack/update-dependencies-script/README.md (1)
10-17: Add language specifiers to fenced code blocks.The fenced code blocks are missing language specifiers for better syntax highlighting and linting compliance.
Apply this diff:
Example: -``` +```makefile ARGO_CD_TARGET_VERSION ?= 3.1.8 ``` Then run the script: -``` +```bash make update-dependencies ```hack/update-dependencies-script/main.go (1)
481-504: Consider cleaning up the temporary directory.The cloned Argo CD repository in the temp directory is never cleaned up. This could leave large temporary directories on disk after repeated runs.
Consider returning a cleanup function or adding a
deferinmain()to remove the temp directory:// In main(), after cloning: defer os.RemoveAll(filepath.Dir(argoCDRepoRoot))Or return a cleanup function from
cloneArgoCDRepoIntoTempDir:func cloneArgoCDRepoIntoTempDir(latestReleaseVersionTag string) (string, func(), error) { tmpDir, err := os.MkdirTemp("", "argo-cd-src") if err != nil { return "", nil, err } cleanup := func() { os.RemoveAll(tmpDir) } // ... rest of function return newWorkDir, cleanup, nil }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
hack/update-dependencies-script/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/lint.yaml(2 hunks)Makefile(2 hunks)hack/update-dependencies-script/README.md(1 hunks)hack/update-dependencies-script/go.mod(1 hunks)hack/update-dependencies-script/main.go(1 hunks)hack/update-dependencies-script/run.sh(1 hunks)hack/update-dependencies-script/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- hack/update-dependencies-script/utils.go
- hack/update-dependencies-script/go.mod
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 375-375: Missing required phony target "all"
(minphony)
[warning] 375-375: Missing required phony target "clean"
(minphony)
[warning] 375-375: Missing required phony target "test"
(minphony)
🪛 markdownlint-cli2 (0.18.1)
hack/update-dependencies-script/README.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
hack/update-dependencies-script/run.sh
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build
- GitHub Check: Run golangci-lint and gosec
- GitHub Check: Run end-to-end tests (v1.27.1)
- GitHub Check: build-operator
🔇 Additional comments (1)
Makefile (1)
8-13: LGTM!The
ARGO_CD_TARGET_VERSIONvariable is well-documented with clear instructions about the version format (novprefix) and the follow-up action (make update-dependencies).
| - name: Checkout code | ||
| uses: actions/checkout@v5 | ||
| - name: "Call update dependencies Makefile target" | ||
| run: | | ||
| make update-dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Go setup step and use consistent checkout version.
The new job is missing the actions/setup-go step required by the dependency update script (which runs go run .). Also, this job uses actions/checkout@v5 while lint_code uses @v6.
Apply this diff:
verify_argo_cd_has_updated_dependencies:
name: Verify Argo CD has updated dependencies, for the given target version
runs-on: ubuntu-latest
steps:
- name: Checkout code
- uses: actions/checkout@v5
+ uses: actions/checkout@v6
+ - name: Set up Go
+ uses: actions/setup-go@v6
+ with:
+ go-version-file: 'go.mod'
- name: "Call update dependencies Makefile target"
run: |
make update-dependencies📝 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.
| - name: Checkout code | |
| uses: actions/checkout@v5 | |
| - name: "Call update dependencies Makefile target" | |
| run: | | |
| make update-dependencies | |
| - name: Checkout code | |
| uses: actions/checkout@v6 | |
| - name: Set up Go | |
| uses: actions/setup-go@v6 | |
| with: | |
| go-version-file: 'go.mod' | |
| - name: "Call update dependencies Makefile target" | |
| run: | | |
| make update-dependencies |
🤖 Prompt for AI Agents
.github/workflows/lint.yaml around lines 36 to 40: the job currently checks out
the repo with actions/checkout@v5 and runs make update-dependencies without
setting up Go; add an actions/setup-go step before running the Make target and
update the checkout step to actions/checkout@v6 to match lint_code. Ensure
setup-go is configured with the repository's Go version (or a sensible default)
and appears before the make update-dependencies run.
| exitWithError(fmt.Errorf("unable to dex_test.go: %v", err)) | ||
| return "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect error message.
The error message is truncated/incorrect. It should describe what file failed to read.
Apply this diff:
- exitWithError(fmt.Errorf("unable to dex_test.go: %v", err))
+ exitWithError(fmt.Errorf("unable to read Makefile: %v", err))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exitWithError(fmt.Errorf("unable to dex_test.go: %v", err)) | |
| return "" | |
| exitWithError(fmt.Errorf("unable to read Makefile: %v", err)) | |
| return "" |
🤖 Prompt for AI Agents
In hack/update-dependencies-script/main.go around lines 78-79, the error message
passed to exitWithError is incorrect/truncated; change the fmt.Errorf string to
clearly state the file read failure (e.g. "unable to read dex_test.go: %v") so
the log accurately describes which file failed to read, and keep the existing
error value substitution.
| if err := os.WriteFile(fileToUpdate, []byte(newContent), 0600); err != nil { | ||
| exitWithError(fmt.Errorf("unable to update build/util/Dockefile: %v", err)) | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in error message.
The error message has a typo: "Dockefile" should be "Dockerfile". Also, the message references build/util/Dockerfile but the function is updating dex_test.go.
Apply this diff:
if err := os.WriteFile(fileToUpdate, []byte(newContent), 0600); err != nil {
- exitWithError(fmt.Errorf("unable to update build/util/Dockefile: %v", err))
+ exitWithError(fmt.Errorf("unable to update dex_test.go: %v", err))
return
}📝 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.
| if err := os.WriteFile(fileToUpdate, []byte(newContent), 0600); err != nil { | |
| exitWithError(fmt.Errorf("unable to update build/util/Dockefile: %v", err)) | |
| return | |
| if err := os.WriteFile(fileToUpdate, []byte(newContent), 0600); err != nil { | |
| exitWithError(fmt.Errorf("unable to update dex_test.go: %v", err)) | |
| return |
🤖 Prompt for AI Agents
In hack/update-dependencies-script/main.go around lines 142-144, the error
string contains a typo ("Dockefile") and references the wrong hard-coded path;
change the fmt.Errorf call to use the actual target filename instead of
"build/util/Dockerfile" and fix the typo by formatting the message with the
fileToUpdate variable (e.g. "unable to update %s: %v", fileToUpdate, err) so the
error reports the correct file name and error details.
| make update-dependencies | ||
| ``` | ||
|
|
||
| See `hack/update-dependencies-script/main.go` for list dependencies that are updated. No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar fix.
Missing word "of" in the sentence.
Apply this diff:
-See `hack/update-dependencies-script/main.go` for list dependencies that are updated.
+See `hack/update-dependencies-script/main.go` for list of dependencies that are updated.📝 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.
| See `hack/update-dependencies-script/main.go` for list dependencies that are updated. | |
| See `hack/update-dependencies-script/main.go` for list of dependencies that are updated. |
🤖 Prompt for AI Agents
hack/update-dependencies-script/README.md around line 19: the sentence "See
`hack/update-dependencies-script/main.go` for list dependencies that are
updated." is missing the word "of"; update the line to read "See
`hack/update-dependencies-script/main.go` for list of dependencies that are
updated." to fix the grammar.
What type of PR is this?
/kind chore
What does this PR do / why we need it:
Have you updated the necessary documentation?
Summary by CodeRabbit
New Features
make update-dependenciescommand to update Argo CD dependencies and related artifacts.Chores
✏️ Tip: You can customize this high-level summary in your review settings.