Conversation
WalkthroughThis pull request adds an OpenAPI 3.1.0 specification for the Config DB API, extends API data models with pretty-printing methods and redirection metadata fields, introduces CLI flags for controlling result export and database persistence, enhances change processing rules with target redirection capabilities, and adds debugging instrumentation across scraper processors. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/v1/interface.go (1)
194-196: Bug: Incorrectappendusage with format string.Line 195 uses
append(s, "warning=%s", w)which passes two arguments to append instead of a formatted string. This will cause a compilation error.🐛 Proposed fix
for _, w := range summary.Warnings { - s = append(s, "warning=%s", w) + s = append(s, fmt.Sprintf("warning=%s", w)) }
🤖 Fix all issues with AI agents
In `@api/v1/common.go`:
- Around line 430-437: The call to clicky.RedactSecretValues() in the
Connection.Pretty method is incorrect and unnecessary because
sanitizeEndpoints(c.Connection) already redacts credentials; either remove the
call entirely from func (c Connection) Pretty() or, if you intended to register
the connection string globally for clicky, call
clicky.RedactSecretValues(c.Connection) (pass c.Connection as an argument)
instead; update the Connection.Pretty function to use only
sanitizeEndpoints(c.Connection) or to pass c.Connection into
clicky.RedactSecretValues accordingly.
In `@api/v1/interface.go`:
- Around line 815-831: The PrettyRow method on ScrapeResult is printing debug
output via two clicky.MustPrint calls during row construction; remove the two
clicky.MustPrint(...) lines from ScrapeResult.PrettyRow so the function only
builds and returns the row (leaving the clicky.Text and Error handling intact)
to avoid side effects from printing during data formatting.
- Around line 1020-1029: In the loop over r.Changes where you build
models.ConfigChange, fix the type mismatch on CreatedAt by dereferencing the
*time.Time safely: set CreatedAt using lo.FromPtr(change.CreatedAt) (or
equivalent safe dereference) instead of assigning the pointer directly; update
the struct construction in the block that creates configChange inside the for _,
change := range r.Changes loop so models.ConfigChange.CreatedAt receives a
time.Time value.
In `@cmd/run.go`:
- Around line 29-30: The variable export is declared and bound to a CLI flag but
never used; either remove the export declaration and its flag binding entirely
(same-scope symbol: export) to avoid dead code, or implement the intended export
behavior by adding a conditional check where output/save logic runs (e.g., in
the function handling the run command) that uses export together with save to
gate printing vs writing to file; update the flag parsing/usage to reference
export and ensure any code paths that should perform export call the existing
save/export routines rather than only using save.
🧹 Nitpick comments (3)
scrapers/sql/sql.go (1)
61-74: Debug instrumentation looks good, but consider logging the format error.The debug logging blocks are properly guarded by
ctx.IsDebug(). However, the error fromclicky.Format(rows)on line 72 is silently ignored, which could hide formatting issues during debugging.♻️ Suggested improvement
if ctx.IsDebug() { - d, _ := clicky.Format(rows) - ctx.Logger.Infof("SQL response: %d rows\n%s", rows.Count, d) + d, err := clicky.Format(rows) + if err != nil { + ctx.Logger.Infof("SQL response: %d rows (format error: %v)", rows.Count, err) + } else { + ctx.Logger.Infof("SQL response: %d rows\n%s", rows.Count, d) + } }api/v1/common.go (1)
69-95: Consider extracting shared logic to reduce duplication.
PrettyShort()andPretty()have nearly identical structures, differing only in whether the content is truncated. You could extract a helper method to reduce duplication.♻️ Suggested refactor
+func (s Script) pretty(truncate bool) api.Text { + t := clicky.Text("") + content := func(val string) string { + if truncate { + return lo.Ellipsis(val, 200) + } + return val + } + if s.GoTemplate != "" { + t = t.Append("go: ", "text-green-600").Append(clicky.CodeBlock("go", content(s.GoTemplate))) + } else if s.JSONPath != "" { + t = t.Append("jsonpath: ", "text-blue-600").Append(clicky.CodeBlock("jsonpath", content(s.JSONPath))) + } else if s.Expression != "" { + t = t.Append("expr: ", "text-yellow-600").Append(clicky.CodeBlock("cel", content(s.Expression))) + } else if s.Javascript != "" { + t = t.Append("js: ", "text-purple-600").Append(clicky.CodeBlock("javascript", content(s.Javascript))) + } + return t +} + func (s Script) PrettyShort() api.Text { - t := clicky.Text("") - if s.GoTemplate != "" { - t = t.Append("go: ", "text-green-600").Append(clicky.CodeBlock("go", lo.Ellipsis(s.GoTemplate, 200))) - } else if s.JSONPath != "" { - t = t.Append("jsonpath: ", "text-blue-600").Append(clicky.CodeBlock("jsonpath", lo.Ellipsis(s.JSONPath, 200))) - } else if s.Expression != "" { - t = t.Append("expr: ", "text-yellow-600").Append(clicky.CodeBlock("cel", lo.Ellipsis(s.Expression, 200))) - } else if s.Javascript != "" { - t = t.Append("js: ", "text-purple-600").Append(clicky.CodeBlock("javascript", lo.Ellipsis(s.Javascript, 200))) - } - return t + return s.pretty(true) } func (s Script) Pretty() api.Text { - t := clicky.Text("") - if s.GoTemplate != "" { - t = t.Append("go: ", "text-green-600").Append(clicky.CodeBlock("go", s.GoTemplate)) - } else if s.JSONPath != "" { - t = t.Append("jsonpath: ", "text-blue-600").Append(clicky.CodeBlock("jsonpath", s.JSONPath)) - } else if s.Expression != "" { - t = t.Append("expr: ", "text-yellow-600").Append(clicky.CodeBlock("cel", s.Expression)) - } else if s.Javascript != "" { - t = t.Append("js: ", "text-purple-600").Append(clicky.CodeBlock("javascript", s.Javascript)) - } - return t + return s.pretty(false) }api/openapi.yaml (1)
1-30: Consider adding global security definitions.The OpenAPI spec lacks a global
securityfield. While debug endpoints note localhost restrictions, and health/metrics endpoints may intentionally be public, consider adding explicit security schemes for endpoints that require authentication (e.g.,/run/{id},/debug/property).♻️ Example security configuration
components: securitySchemes: bearerAuth: type: http scheme: bearer bearerFormat: JWT security: - bearerAuth: []Then override with
security: []for explicitly public endpoints like/live,/ready, and/metrics.
| func (c Connection) Pretty() api.Text { | ||
| t := clicky.Text("") | ||
| if c.Connection != "" { | ||
| clicky.RedactSecretValues() | ||
| t = t.Append(sanitizeEndpoints(c.Connection)) | ||
| } | ||
| return t | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and examine the actual file
echo "=== Examining api/v1/common.go around lines 430-437 ==="
if [ -f "api/v1/common.go" ]; then
sed -n '425,440p' api/v1/common.go | cat -n
else
echo "File not found at api/v1/common.go"
git ls-files | grep -E "common\.go$" | head -5
fiRepository: flanksource/config-db
Length of output: 665
🏁 Script executed:
#!/bin/bash
# Search for clicky package definition and RedactSecretValues function
echo "=== Searching for clicky package definition ==="
fd "clicky" --type f -e go | head -20
echo -e "\n=== Searching for RedactSecretValues usage in codebase ==="
rg "RedactSecretValues" -A 2 -B 2 --type go | head -50
echo -e "\n=== Searching for clicky imports ==="
rg "import.*clicky|from.*clicky" --type goRepository: flanksource/config-db
Length of output: 423
🏁 Script executed:
#!/bin/bash
# Check for sanitizeEndpoints definition
echo "=== Searching for sanitizeEndpoints definition ==="
rg "func.*sanitizeEndpoints" -A 10 --type go
echo -e "\n=== Checking for clicky package in vendor or go.mod ==="
if [ -f "go.mod" ]; then
grep -i clicky go.mod
fi
if [ -d "vendor" ]; then
find vendor -name "*clicky*" -type d 2>/dev/null
fiRepository: flanksource/config-db
Length of output: 770
🏁 Script executed:
#!/bin/bash
# Let's check how clicky is imported
echo "=== Searching for clicky in imports with broader context ==="
rg "clicky" api/v1/common.go | head -20
echo -e "\n=== Let's view the imports section of the file ==="
head -30 api/v1/common.go | cat -n
echo -e "\n=== Check if clicky is aliased or imported differently ==="
rg "clicky\.|clicky$" api/v1/common.go -B 5 | head -40Repository: flanksource/config-db
Length of output: 3635
🏁 Script executed:
#!/bin/bash
# Let's look for documentation or usage patterns of clicky library
echo "=== Search for all clicky function calls in the codebase ==="
rg "clicky\." --type go | head -50Repository: flanksource/config-db
Length of output: 3142
🏁 Script executed:
#!/bin/bash
# Let's look at more context around the RedactSecretValues call
echo "=== Full Connection.Pretty() method with more context ==="
rg "func \(c Connection\) Pretty" -A 15 --type go api/v1/common.go
echo -e "\n=== Check sanitizeEndpoints full implementation ==="
rg "func sanitizeEndpoints" -A 25 --type go api/v1/common.goRepository: flanksource/config-db
Length of output: 1842
🌐 Web query:
clicky RedactSecretValues golang flanksource github
💡 Result:
I couldn’t find any public, indexable GitHub match for RedactSecretValues inside github.com/flanksource/clicky via web search (it may be unindexed, moved, renamed, or not in the default branch), but the Go module github.com/flanksource/clicky does exist (e.g., pseudo-version v0.0.0-20250820111116-a1cda798d4da). [1]
To locate it reliably, fetch the module locally and grep it:
go get github.com/flanksource/clicky@v0.0.0-20250820111116-a1cda798d4da
# then search in your module cache
grep -R --line-number --fixed-strings "RedactSecretValues" "$(go env GOPATH)/pkg/mod" | headIf you paste the go.mod line (or the commit SHA/tag you’re using), I can help narrow it down further.
[1]
🏁 Script executed:
#!/bin/bash
# Let's check the go.mod file for the exact version
echo "=== clicky version in go.mod ==="
grep "clicky" go.mod
# Try to find if there's a local copy or vendor directory
echo -e "\n=== Looking for clicky in vendor ==="
find vendor -name "*.go" -path "*clicky*" 2>/dev/null | head -20
# Try to search for clicky package source if available
echo -e "\n=== Checking for clicky source in go mod cache ==="
ls -la "$(go env GOPATH)/pkg/mod/github.com/flanksource/clicky"* 2>/dev/null | head -20Repository: flanksource/config-db
Length of output: 356
🏁 Script executed:
#!/bin/bash
# Let's try to get the clicky module and search for RedactSecretValues
echo "=== Fetching clicky module and searching for RedactSecretValues ==="
go get github.com/flanksource/clicky@v1.12.0 2>&1 | head -20
# Search in the go mod cache
echo -e "\n=== Searching for RedactSecretValues in clicky module ==="
find "$(go env GOPATH)/pkg/mod/github.com/flanksource" -name "*.go" 2>/dev/null | xargs grep -l "RedactSecretValues" 2>/dev/null | head -10
# Get the content of the matching files
if [ -n "$(find "$(go env GOPATH)/pkg/mod/github.com/flanksource" -name "*.go" 2>/dev/null | xargs grep -l "RedactSecretValues" 2>/dev/null | head -1)" ]; then
echo -e "\n=== Content of RedactSecretValues function ==="
find "$(go env GOPATH)/pkg/mod/github.com/flanksource" -name "*.go" 2>/dev/null | xargs grep -B 3 -A 10 "func.*RedactSecretValues" 2>/dev/null | head -30
fiRepository: flanksource/config-db
Length of output: 2963
🏁 Script executed:
#!/bin/bash
# Let's see more of the RedactSecretValues context and function signature
echo "=== Full RedactSecretValues function context ==="
grep -B 5 -A 5 "func RedactSecretValues" /home/jailuser/go/pkg/mod/github.com/flanksource/clicky@v1.12.0/format.go
# Search for how RedactSecretValues is used in the config-db repo with arguments
echo -e "\n=== How RedactSecretValues is called in config-db ==="
rg "RedactSecretValues" -B 2 -A 2 --type go
# Let's also check if there are other patterns of secret redaction in the codebase
echo -e "\n=== Search for patterns of how secrets are handled ==="
rg "RedactSecrets|redact|password" --type go | grep -i "func\|redact" | head -20Repository: flanksource/config-db
Length of output: 976
Remove or fix the clicky.RedactSecretValues() call—it's being called incorrectly without arguments and appears unnecessary.
The clicky.RedactSecretValues() function accepts variadic string arguments and modifies global logging state; when called without arguments (as it is here), it doesn't affect the connection string redaction. The actual redaction is already handled by sanitizeEndpoints(), which removes passwords and user credentials. Either remove this call entirely or, if intended to globally register the connection string for secret filtering, pass it as an argument: clicky.RedactSecretValues(c.Connection).
🤖 Prompt for AI Agents
In `@api/v1/common.go` around lines 430 - 437, The call to
clicky.RedactSecretValues() in the Connection.Pretty method is incorrect and
unnecessary because sanitizeEndpoints(c.Connection) already redacts credentials;
either remove the call entirely from func (c Connection) Pretty() or, if you
intended to register the connection string globally for clicky, call
clicky.RedactSecretValues(c.Connection) (pass c.Connection as an argument)
instead; update the Connection.Pretty function to use only
sanitizeEndpoints(c.Connection) or to pass c.Connection into
clicky.RedactSecretValues accordingly.
| func (s ScrapeResult) PrettyRow(options interface{}) map[string]api.Text { | ||
| row := make(map[string]api.Text) | ||
| row["ID"] = clicky.Text(s.ID) | ||
| row["Name"] = clicky.Text(s.Name) | ||
| row["Type"] = clicky.Text(s.Type) | ||
| row["Health"] = clicky.Text(string(s.Health)) | ||
| row["Status"] = clicky.Text(s.Status) | ||
| if s.Error != nil { | ||
| row["Error"] = clicky.Text(s.Error.Error()) | ||
| } else { | ||
| row["Error"] = clicky.Text("") | ||
| } | ||
| clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{JSON: true}) | ||
| clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{Pretty: true}) | ||
|
|
||
| return row | ||
| } |
There was a problem hiding this comment.
Remove debug print statements.
Lines 827-828 contain clicky.MustPrint calls that output directly during row construction. These appear to be debug artifacts and should be removed as they cause unintended side effects.
🐛 Proposed fix
func (s ScrapeResult) PrettyRow(options interface{}) map[string]api.Text {
row := make(map[string]api.Text)
row["ID"] = clicky.Text(s.ID)
row["Name"] = clicky.Text(s.Name)
row["Type"] = clicky.Text(s.Type)
row["Health"] = clicky.Text(string(s.Health))
row["Status"] = clicky.Text(s.Status)
if s.Error != nil {
row["Error"] = clicky.Text(s.Error.Error())
} else {
row["Error"] = clicky.Text("")
}
- clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{JSON: true})
- clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{Pretty: true})
-
return row
}📝 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 (s ScrapeResult) PrettyRow(options interface{}) map[string]api.Text { | |
| row := make(map[string]api.Text) | |
| row["ID"] = clicky.Text(s.ID) | |
| row["Name"] = clicky.Text(s.Name) | |
| row["Type"] = clicky.Text(s.Type) | |
| row["Health"] = clicky.Text(string(s.Health)) | |
| row["Status"] = clicky.Text(s.Status) | |
| if s.Error != nil { | |
| row["Error"] = clicky.Text(s.Error.Error()) | |
| } else { | |
| row["Error"] = clicky.Text("") | |
| } | |
| clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{JSON: true}) | |
| clicky.MustPrint([]map[string]api.Text{row}, clicky.FormatOptions{Pretty: true}) | |
| return row | |
| } | |
| func (s ScrapeResult) PrettyRow(options interface{}) map[string]api.Text { | |
| row := make(map[string]api.Text) | |
| row["ID"] = clicky.Text(s.ID) | |
| row["Name"] = clicky.Text(s.Name) | |
| row["Type"] = clicky.Text(s.Type) | |
| row["Health"] = clicky.Text(string(s.Health)) | |
| row["Status"] = clicky.Text(s.Status) | |
| if s.Error != nil { | |
| row["Error"] = clicky.Text(s.Error.Error()) | |
| } else { | |
| row["Error"] = clicky.Text("") | |
| } | |
| return row | |
| } |
🤖 Prompt for AI Agents
In `@api/v1/interface.go` around lines 815 - 831, The PrettyRow method on
ScrapeResult is printing debug output via two clicky.MustPrint calls during row
construction; remove the two clicky.MustPrint(...) lines from
ScrapeResult.PrettyRow so the function only builds and returns the row (leaving
the clicky.Text and Error handling intact) to avoid side effects from printing
during data formatting.
| for _, change := range r.Changes { | ||
| configChange := models.ConfigChange{ | ||
| ChangeType: change.ChangeType, | ||
| Severity: models.Severity(change.Severity), | ||
| Source: change.Source, | ||
| Summary: change.Summary, | ||
| CreatedAt: change.CreatedAt, | ||
| } | ||
| full.Changes = append(full.Changes, configChange) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the models.ConfigChange struct definition and check CreatedAt type
ast-grep --pattern $'type ConfigChange struct {
$$$
}'Repository: flanksource/config-db
Length of output: 2088
🏁 Script executed:
# Also search for ChangeResult struct to verify the source type
ast-grep --pattern $'type ChangeResult struct {
$$$
}'Repository: flanksource/config-db
Length of output: 1910
🏁 Script executed:
# Check the actual assignment in the file to see context
sed -n '1015,1035p' api/v1/interface.goRepository: flanksource/config-db
Length of output: 662
🏁 Script executed:
# Search for lo.FromPtr usage to see if it's available in the codebase
rg "FromPtr" --type goRepository: flanksource/config-db
Length of output: 22734
Fix type mismatch for CreatedAt field assignment.
change.CreatedAt is a *time.Time pointer, but models.ConfigChange.CreatedAt expects time.Time. Directly assigning the pointer will cause a type error. Use lo.FromPtr() to safely dereference:
Fix
for _, change := range r.Changes {
configChange := models.ConfigChange{
ChangeType: change.ChangeType,
Severity: models.Severity(change.Severity),
Source: change.Source,
Summary: change.Summary,
- CreatedAt: change.CreatedAt,
+ CreatedAt: lo.FromPtr(change.CreatedAt),
}
full.Changes = append(full.Changes, configChange)
}🤖 Prompt for AI Agents
In `@api/v1/interface.go` around lines 1020 - 1029, In the loop over r.Changes
where you build models.ConfigChange, fix the type mismatch on CreatedAt by
dereferencing the *time.Time safely: set CreatedAt using
lo.FromPtr(change.CreatedAt) (or equivalent safe dereference) instead of
assigning the pointer directly; update the struct construction in the block that
creates configChange inside the for _, change := range r.Changes loop so
models.ConfigChange.CreatedAt receives a time.Time value.
| var export bool | ||
| var save bool |
There was a problem hiding this comment.
The export flag is defined but never used.
The export variable is declared and bound to a flag (line 179), but it's never checked in the code logic. Either implement the intended behavior or remove the unused flag.
🐛 Proposed fix (if export should gate printing/file export)
- if outputDir != "" {
+ if export && outputDir != "" {
for _, result := range results {
if err := exportResource(result, outputDir); err != nil {
return fmt.Errorf("failed to export results %v", err)
}
}
logger.Infof("Exported %d resources to %s (%s)", len(results), outputDir, timer.End())
- } else {
+ } else if export {
clicky.MustPrint(all.Configs)
clicky.MustPrint(all.Changes)
clicky.MustPrint(all.Analysis)
clicky.MustPrint(all.Relationships)
-
}🤖 Prompt for AI Agents
In `@cmd/run.go` around lines 29 - 30, The variable export is declared and bound
to a CLI flag but never used; either remove the export declaration and its flag
binding entirely (same-scope symbol: export) to avoid dead code, or implement
the intended export behavior by adding a conditional check where output/save
logic runs (e.g., in the function handling the run command) that uses export
together with save to gate printing vs writing to file; update the flag
parsing/usage to reference export and ensure any code paths that should perform
export call the existing save/export routines rather than only using save.
1 participant
Summary by CodeRabbit
New Features
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.