Merged
Conversation
BenchstatBase: |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
WalkthroughThis pull request implements conditional field preservation during connection hydration, adds AWS SigV4 support to HTTP connections, introduces extensive OpenAPI schema definitions for execution and notification capabilities, updates health check schemas with descriptive metadata and dependency ordering, and refactors PostgREST binary execution. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPConnection
participant AWSSigV4
participant AWSConfig
participant HTTPServer
Client->>HTTPConnection: Create with AWSSigV4
HTTPConnection->>AWSSigV4: Store configuration
HTTPConnection->>AWSConfig: Initialize cached AWS config
HTTPConnection->>HTTPConnection: Pretty() display config
Client->>HTTPConnection: Execute request
HTTPConnection->>AWSConfig: Load AWS credentials
AWSConfig->>HTTPServer: Sign request with SigV4
HTTPServer->>HTTPServer: Verify Authorization header
HTTPServer-->>HTTPConnection: Response
HTTPConnection-->>Client: Return response
sequenceDiagram
participant Receiver
participant Connection
participant Hydrator
participant Existing as Existing Values
Receiver->>Hydrator: Trigger hydration
Hydrator->>Existing: Check if field empty/nil
alt Field is empty/nil
Hydrator->>Connection: Read value from connection
Connection-->>Hydrator: Return value
Hydrator->>Receiver: Assign new value
else Field already set
Hydrator->>Receiver: Preserve existing value
end
Hydrator-->>Receiver: Hydration complete
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
schema/openapi/config_logs.schema.json (1)
497-524:⚠️ Potential issue | 🟠 MajorRequire at least one OpenSearch endpoint source (
connectionoraddress).
OpenSearchConfigcan now validate without either field, which defers a hard failure to runtime instead of schema validation.Suggested schema fix
"OpenSearchConfig": { "properties": { "connection": { "type": "string" }, "address": { "type": "string" }, ... }, "additionalProperties": false, "type": "object", + "anyOf": [ + { "required": ["connection"] }, + { "required": ["address"] } + ], "required": [ "index", "query" ] },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/config_logs.schema.json` around lines 497 - 524, OpenSearchConfig currently allows neither endpoint field to be present; update the JSON schema so at least one of the endpoint sources is required by adding an anyOf (or oneOf) clause that enforces either "connection" or "address" is present (e.g., anyOf: [{required:["connection"]},{required:["address"]}). Modify the schema that defines the OpenSearchConfig properties (fields "connection" and "address" and the surrounding object schema) to include this anyOf/oneOf constraint while keeping additionalProperties:false and the existing required list for "index" and "query".connection/http.go (1)
352-360:⚠️ Potential issue | 🔴 CriticalDo not mutate
rt.BaseinsideRoundTrip.
RoundTripmay run concurrently; mutatingrt.Baseper request can stack middleware repeatedly and introduce race-prone behavior.🔧 Proposed fix
func (rt *httpConnectionRoundTripper) RoundTrip(req *netHTTP.Request) (*netHTTP.Response, error) { conn := rt.HTTPConnection + base := rt.Base if !conn.HTTPBasicAuth.IsEmpty() { req.SetBasicAuth(conn.HTTPBasicAuth.GetUsername(), conn.HTTPBasicAuth.GetPassword()) } else if !conn.Bearer.IsEmpty() { req.Header.Set(echo.HeaderAuthorization, "Bearer "+conn.Bearer.ValueStatic) } else if !conn.OAuth.IsEmpty() { oauthTransport := middlewares.NewOauthTransport(middlewares.OauthConfig{ ClientID: conn.OAuth.ClientID.String(), ClientSecret: conn.OAuth.ClientSecret.String(), TokenURL: conn.OAuth.TokenURL, Params: conn.OAuth.Params, Scopes: conn.OAuth.Scopes, }) - rt.Base = oauthTransport.RoundTripper(rt.Base) + base = oauthTransport.RoundTripper(base) } @@ if conn.AWSSigV4 != nil && conn.awsConfig != nil { - rt.Base = middlewares.NewAWSSigv4Transport(middlewares.AWSSigv4Config{ + base = middlewares.NewAWSSigv4Transport(middlewares.AWSSigv4Config{ Region: conn.awsConfig.Region, Service: conn.AWSSigV4.Service, CredentialsProvider: conn.awsConfig.Credentials, - }, rt.Base) + }, base) } - return rt.Base.RoundTrip(req) + return base.RoundTrip(req) }Also applies to: 372-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connection/http.go` around lines 352 - 360, The current RoundTrip mutates rt.Base by calling rt.Base = oauthTransport.RoundTripper(rt.Base), which is unsafe because RoundTrip can run concurrently and will repeatedly stack middleware; instead, do not assign back to rt.Base—in RoundTrip capture the current base transport into a local variable (e.g., base := rt.Base), build the wrapped transport via oauthTransport.RoundTripper(base), and use that local wrapped transport to perform the request; apply the same pattern for other middleware-wrapping instances (e.g., the other oauthTransport creation) so no shared rt.Base is ever mutated inside RoundTrip.
🟡 Minor comments (16)
schema/openapi/health_helm.schema.json-96-98 (1)
96-98:⚠️ Potential issue | 🟡 MinorComplete the
namespacedescription text.The description currently ends with
e.g.and reads incomplete in generated docs.✏️ Proposed wording fix
"namespace": { "type": "string", - "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to place the check in when it differs from the namespace where the Canary is defined." },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_helm.schema.json` around lines 96 - 98, The "namespace" property description for the schema property named "namespace" is incomplete (ends with "e.g."); update the description string in health_helm.schema.json for the "namespace" field to be a complete sentence and include a short example (for instance: "Namespace to insert the check into, if different from the namespace the canary is defined in, e.g. 'monitoring'"). Ensure punctuation and grammar are fixed ("from" instead of "to" where appropriate) and keep the wording concise for generated docs.schema/openapi/health_mongodb.schema.json-127-130 (1)
127-130:⚠️ Potential issue | 🟡 MinorTruncated description for
namespacefield.The description
"Namespace to insert the check into, if different to the namespace the canary is defined, e.g."ends abruptly with", e.g.". It should either complete the example or drop the trailing clause.✏️ Proposed fix
- "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g. monitoring"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_mongodb.schema.json` around lines 127 - 130, The "namespace" property's description in the OpenAPI JSON is truncated and ends with an unfinished "e.g."; update the description for the "namespace" field (property name: namespace) to either remove the trailing "e.g." or complete it with a concrete example—e.g., "Namespace to insert the check into, if different from the namespace the canary is defined in, e.g. 'monitoring'."—so the sentence reads as a full, clear description.schema/openapi/health_databasebackupcheck.schema.json-50-53 (1)
50-53:⚠️ Potential issue | 🟡 MinorFix incomplete
namespacedescription text.Line 52 ends with “e.g.” but does not provide an example, so the field docs read as unfinished. Please complete the sentence or remove the trailing “e.g.”.
✏️ Suggested wording
"namespace": { "type": "string", - "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into if different from the namespace where the canary is defined." },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_databasebackupcheck.schema.json` around lines 50 - 53, The "namespace" property description is incomplete (it ends with "e.g."); update the "namespace" field description in the schema to a complete sentence or remove the trailing "e.g." — for example: "Namespace to insert the check into, if different from the namespace the canary is defined in (e.g., 'prod' or 'staging')." Locate and edit the "namespace" property in the health_databasebackupcheck JSON schema to replace the unfinished text with the chosen complete wording.schema/openapi/health_gitProtocol.schema.json-84-86 (1)
84-86:⚠️ Potential issue | 🟡 MinorComplete the
namespacedescription text.The sentence ends with “e.g.” and renders as incomplete in docs.
✏️ Suggested wording fix
- "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into, if different from the namespace where the canary is defined."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_gitProtocol.schema.json` around lines 84 - 86, The "namespace" schema description in schema/openapi/health_gitProtocol.schema.json is incomplete; update the "description" value for the namespace property to be a full sentence and include a concrete example (for instance: "Namespace to insert the check into, if different from the namespace the canary is defined in (e.g., 'team-a' or 'default')."). Locate the "namespace" property in the schema and replace the existing truncated text with the completed sentence and example.schema/openapi/health_pubsub.schema.json-181-183 (1)
181-183:⚠️ Potential issue | 🟡 Minor
namespacedescription appears truncated — "e.g." with no example.The description
"Namespace to insert the check into, if different to the namespace the canary is defined, e.g."ends mid-sentence. Either supply the example (e.g.,"e.g. monitoring") or drop the trailing "e.g."✏️ Proposed fix
- "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g. monitoring"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_pubsub.schema.json` around lines 181 - 183, The "namespace" property description is truncated ("e.g." with no example); update the description for the namespace property in the schema to either remove the trailing "e.g." or complete the example (for example: "Namespace to insert the check into, if different to the namespace the canary is defined, e.g. monitoring"). Edit the description string for the "namespace" property so it reads as a complete sentence and includes a concrete example if you choose to keep "e.g.".schema/openapi/health_junit.schema.json-49-50 (1)
49-50:⚠️ Potential issue | 🟡 MinorIncomplete
namespacedescription — truncated at "e.g."The description ends mid-sentence with
"e.g."and no example follows:"Namespace to insert the check into, if different to the namespace the canary is defined, e.g."
✏️ Suggested fix
- "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g. production"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_junit.schema.json` around lines 49 - 50, The "namespace" property description in health_junit.schema.json is truncated ("e.g.") — update the "description" for the namespace property to complete the sentence and include an example (for example: "Namespace to insert the check into, if different to the namespace the canary is defined, e.g. 'monitoring' or 'default'."), ensuring the text remains a single string value for the "namespace" schema entry so consumers see a clear explanation and example.schema/openapi/health_junit.schema.json-57-58 (1)
57-58:⚠️ Potential issue | 🟡 Minor
$refwith siblingdescription— compliant with Draft 2020-12, but tooling support remains incompleteBoth
labels(Line 57) andrelationships(Line 95) use$refalongside a siblingdescriptionkeyword. JSON Schema Draft 2020-12 allows this (unlike earlier drafts), and the file correctly declares"$schema": "https://json-schema.org/draft/2020-12/schema". However, as of 2024–2025, several code generators and schema tools do not yet fully support this pattern:zod-to-json-schemastates "2020-12 not yet officially supported", TypeSpec is still adding 2020-12 features, andjsonschema2pojohas open issues around modern$refsemantics. Real-world failures have been reported (e.g., MCP TypeScript SDK generating draft-07 schemas that break clients expecting 2020-12).To ensure compatibility with older/incomplete tooling, consider wrapping
$refinallOf:"allOf": [ { "$ref": "#/$defs/Labels" }, { "description": "Labels for the check" } ]This pattern works across all draft versions and avoids sibling-stripping issues.
Also applies to: 95-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_junit.schema.json` around lines 57 - 58, The schema uses sibling descriptions alongside "$ref" for the properties labels and relationships (declared under "$defs" in this JSON Schema); to maintain compatibility with tools that strip sibling keywords, replace the direct "$ref" sibling pattern for the labels and relationships property definitions with an allOf wrapper that contains the $ref entry and a separate schema object holding the description (i.e., convert the labels and relationships property definitions to use allOf with one element referencing "#/$defs/Labels" or the appropriate def and another element giving the description), ensuring the schema still targets Draft 2020-12 but avoids tooling that removes sibling keywords.schema/openapi/scrape_plugin.schema.json-114-118 (1)
114-118:⚠️ Potential issue | 🟡 Minor
FieldsV1as empty object is consistent with theTimepattern — but note the same caveat.
FieldsV1in Kubernetes is an opaque JSON blob (arbitrary structure). Defining it withadditionalProperties: falsemeans any actual managed-fields data would be rejected. If this schema is used for validation of real K8s-like resources, this will be a problem. If it's only used for UI/documentation purposes, this is acceptable.If validation of real data is needed
"FieldsV1": { - "properties": {}, - "additionalProperties": false, - "type": "object" + "type": "object" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/scrape_plugin.schema.json` around lines 114 - 118, The FieldsV1 schema currently sets "additionalProperties": false which forbids arbitrary managed-fields content; update the FieldsV1 definition so it accepts an opaque JSON blob by allowing arbitrary properties (e.g., remove or change "additionalProperties": false to "additionalProperties": true or replace the properties block with a free-form object schema), keeping "type": "object" so real Kubernetes managedFields data won't be rejected; locate the FieldsV1 schema entry and make this change.context/envvar.go-60-61 (1)
60-61:⚠️ Potential issue | 🟡 MinorLog gating and emitted verbosity don’t match.
The condition checks level 2, but the message is emitted at
V(5). At verbosity 2-4 this block runs but still won’t log.Suggested fix
- } else if ctx.Logger.IsLevelEnabled(2) && properties.On(false, "envvar.lookup.log") { - ctx.Logger.V(5).Infof("lookup[%s] %s => %s", input.Name, source, logger.PrintableSecret(value)) + } else if ctx.Logger.IsLevelEnabled(2) && properties.On(false, "envvar.lookup.log") { + ctx.Logger.V(2).Infof("lookup[%s] %s => %s", input.Name, source, logger.PrintableSecret(value)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@context/envvar.go` around lines 60 - 61, The log gating and emission verbosity are mismatched: the condition uses ctx.Logger.IsLevelEnabled(2) but the message is emitted with ctx.Logger.V(5).Infof, so align them by changing the gate to ctx.Logger.IsLevelEnabled(5) (or alternatively change V(5) to V(2)); update the condition that currently reads ctx.Logger.IsLevelEnabled(2) && properties.On(false, "envvar.lookup.log") so it uses ctx.Logger.IsLevelEnabled(5) && properties.On(false, "envvar.lookup.log") to ensure the Infof call in ctx.Logger.V(5).Infof actually executes when expected.schema/openapi/health_restic.schema.json-241-243 (1)
241-243:⚠️ Potential issue | 🟡 MinorClean up credential-like example text and typo in descriptions.
Line 242 includes
user:pass, which can trigger secret scanners unnecessarily, and Line 254 has a typo (reposiotry). Please switch to placeholders and corrected wording.✏️ Suggested text update
- "description": "Repository The restic repository path eg: rest:https://user:pass@host:8000/ or rest:https://host:8000/ or s3:s3.amazonaws.com/bucket_name" + "description": "Repository path, e.g. rest:https://<user>:<password>@host:8000/, rest:https://host:8000/, or s3:s3.amazonaws.com/bucket_name" - "description": "CheckIntegrity when enabled will check the Integrity and consistency of the restic reposiotry" + "description": "CheckIntegrity when enabled checks the integrity and consistency of the restic repository"Also applies to: 253-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_restic.schema.json` around lines 241 - 243, Update the restic repository description strings in schema/openapi/health_restic.schema.json to remove credential-like examples and fix the typo: replace occurrences of "user:pass" with a neutral placeholder such as "<username>:<password>" (or "{username}:{password}") and correct "reposiotry" to "repository"; apply the same wording changes to the other occurrence mentioned (lines ~253-255) so both descriptions use the cleaned placeholder and the corrected spelling.schema/openapi/health_dockerPull.schema.json-62-65 (1)
62-65:⚠️ Potential issue | 🟡 MinorTruncated description for
namespace.The description ends with "e.g." suggesting the sentence was cut short — likely missing an example or trailing clause. If these schemas are auto-generated, the source Go struct doc comment should be fixed.
"Namespace to insert the check into, if different to the namespace the canary is defined, e.g."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_dockerPull.schema.json` around lines 62 - 65, The "namespace" property description in schema/openapi/health_dockerPull.schema.json is truncated ("..., e.g.") — update the description for the "namespace" field to be a complete sentence and include the intended example or clause (e.g., "Namespace to insert the check into, if different from the namespace the canary is defined in, e.g. 'prod' or 'staging'") or, if the schema is generated, fix the source Go struct doc comment for the Namespace field so the generator emits the full description; ensure the description string for "namespace" is clear and no longer ends with an unfinished "e.g.".schema/openapi/health_mysql.schema.json-162-164 (1)
162-164:⚠️ Potential issue | 🟡 MinorTighten two schema descriptions for clarity.
The text at Line 163 is incomplete (
e.g.with no example), and Line 230 has a grammar typo. These appear in generated docs/IDE hints, so worth fixing.Suggested wording
- "description": "Namespace to insert the check into, if different to the namespace the canary is defined, e.g." + "description": "Namespace to insert the check into, if different from the canary namespace." - "description": "Number rows to check for" + "description": "Number of rows to check for"Also applies to: 229-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_mysql.schema.json` around lines 162 - 164, Update the incomplete schema description for the "namespace" property (the description string that currently ends with "e.g.") to include a concrete example (e.g. "e.g. 'monitoring' or 'default'") and make the description a complete sentence; also fix the grammar typo in the other description referenced around lines 229-230 by editing that property's "description" field to read clearly (for example: "Namespace to insert the check into, if different from the namespace the canary is defined in. e.g. 'monitoring'"). Ensure you only modify the JSON "description" values for the relevant schema properties.connection/loki.go-31-36 (1)
31-36:⚠️ Potential issue | 🟡 MinorAvoid materializing empty credential env vars during hydration.
If
conn.Username/conn.Passwordare empty, this now creates non-nil empty pointers, which can alter downstream behavior (e.g., JSON shape). Add a non-empty source guard.Suggested fix
- if c.Username == nil || c.Username.IsEmpty() { + if (c.Username == nil || c.Username.IsEmpty()) && conn.Username != "" { c.Username = &types.EnvVar{ValueStatic: conn.Username} } - if c.Password == nil || c.Password.IsEmpty() { + if (c.Password == nil || c.Password.IsEmpty()) && conn.Password != "" { c.Password = &types.EnvVar{ValueStatic: conn.Password} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connection/loki.go` around lines 31 - 36, The current hydration code unconditionally sets c.Username and c.Password to &types.EnvVar{ValueStatic: conn.Username/conn.Password} even when conn.Username or conn.Password are empty, creating non-nil empty env var objects; change the logic in the hydration path (where c.Username and c.Password are set) to only assign a new types.EnvVar when conn.Username/conn.Password are non-empty (e.g., check conn.Username != "" and conn.Password != "") so you don't materialize empty pointers and alter downstream JSON/behavior.connection/merge_test.go-95-101 (1)
95-101:⚠️ Potential issue | 🟡 MinorAdd assertions for configured merge fields that are currently unverified.
Line 92, Line 145, and Line 276 set values that are central to override/fallback behavior, but the tests don’t assert those outcomes. This leaves key merge paths unprotected.
Proposed test assertion additions
@@ g.Expect(conn.Endpoint).To(gomega.Equal("https://inline-endpoint.aws.com")) g.Expect(conn.Region).To(gomega.Equal("eu-west-1")) + g.Expect(conn.SkipTLSVerify).To(gomega.BeFalse()) @@ g.Expect(conn.Bucket).To(gomega.Equal("inline-bucket")) g.Expect(conn.ObjectPath).To(gomega.Equal("inline/path")) + g.Expect(conn.UsePathStyle).To(gomega.BeFalse()) @@ g.Expect(conn.Domain).To(gomega.Equal("conn-domain")) + g.Expect(conn.Port).To(gomega.Equal(4445)) g.Expect(conn.GetUsername()).To(gomega.Equal("conn-user")) g.Expect(conn.GetPassword()).To(gomega.Equal("conn-pass"))Also applies to: 148-152, 282-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connection/merge_test.go` around lines 95 - 101, The test sets additional merge/override/fallback values earlier in the test but never asserts their outcomes; after calling conn.Populate(ctx) add gomega assertions mirroring the existing pattern (e.g., g.Expect(conn.<FieldName>).To(gomega.Equal("<expected-value>")) or g.Expect(conn.<NestedStruct>.ValueStatic).To(gomega.Equal("<expected-value>"))) for each configured merge field that was set in this test (the same fields configured at the earlier setup sites referenced in the comment), and add the same corresponding assertions in the other two test blocks that set those values (the blocks around the other mentioned setup locations) so each configured override/fallback field is verified.schema/openapi/health_folder.schema.json-135-137 (1)
135-137:⚠️ Potential issue | 🟡 MinorFix incorrect field descriptions (
path,maxCount).Line 136 has a typo (
/path/tp/folder), and Line 159 incorrectly describesmaxCountasMinCount/minimum. These should be corrected to avoid misleading generated docs.✏️ Suggested text fixes
- "description": "Path to folder or object storage, e.g. `s3://<bucket-name>`, `gcs://<bucket-name>`, `/path/tp/folder`" + "description": "Path to folder or object storage, e.g. `s3://<bucket-name>`, `gcs://<bucket-name>`, `/path/to/folder`" - "description": "MinCount the minimum number of files inside the searchPath" + "description": "MaxCount the maximum number of files inside the searchPath"Also applies to: 158-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_folder.schema.json` around lines 135 - 137, Update the JSON schema field descriptions: fix the typo in the "path" description by changing "/path/tp/folder" to "/path/to/folder" and ensure examples still show S3/GCS and local paths; and correct the "maxCount" description (referenced as property "maxCount") so it describes the maximum number of items (not minimum/MinCount) and mentions it is an upper limit. Locate and update the "path" and "maxCount" property descriptions in the schema object to reflect these exact wording fixes.schema/openapi/health_azuredevops.schema.json-92-94 (1)
92-94:⚠️ Potential issue | 🟡 Minor
thresholdMillisdescription conflicts with required contract.Line 93 marks it “(Optional)”, but the schema still requires it at Line 106. Please align wording or required fields to avoid contradictory API docs.
💡 Suggested doc-only fix
- "description": "ThresholdMillis the maximum duration of a Run. (Optional)" + "description": "ThresholdMillis is the maximum duration of a Run."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_azuredevops.schema.json` around lines 92 - 94, The description for the property "thresholdMillis" currently says "(Optional)" but the JSON Schema still lists "thresholdMillis" in the schema's required array; update the schema so documentation and contract match by either removing "thresholdMillis" from the schema's required list or changing the property description to remove "(Optional)". Locate the "thresholdMillis" property in the schema and the surrounding "required" array for the same object (referencing the property name "thresholdMillis" and the schema's "required" array) and make the choice consistent across both places.
🧹 Nitpick comments (8)
schema/openapi/health_dynatrace.schema.json (1)
76-81: TightendependsOnvalidation to reject empty/duplicate entries.At Line [76],
dependsOncurrently accepts empty strings and duplicate check names. Adding basic constraints avoids ambiguous dependency graphs.Suggested schema hardening
"dependsOn": { "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array", + "uniqueItems": true, "description": "DependsOn lists the checks that must complete before this one runs" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_dynatrace.schema.json` around lines 76 - 81, Tighten the dependsOn array schema by rejecting empty strings and duplicates: in the dependsOn definition (the "dependsOn" property where "items" currently has "type":"string"), add constraints on the items such as a non-empty string requirement (e.g., "minLength": 1) and on the array itself enable deduplication validation (e.g., "uniqueItems": true) so empty entries and duplicate check names are disallowed.schema/openapi/health_databasebackupcheck.schema.json (1)
77-83: Consider constrainingdependsOnentries to prevent empty/duplicate values.To make dependency ordering more reliable, it would help to reject empty strings and duplicate names at schema-validation time.
🧩 Suggested schema hardening
"dependsOn": { "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array", + "uniqueItems": true, "description": "DependsOn lists the checks that must complete before this one runs" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_databasebackupcheck.schema.json` around lines 77 - 83, The dependsOn array allows empty or duplicate strings; tighten the JSON Schema for the dependsOn property by adding "minLength": 1 to its items (so empty strings are invalid) and add "uniqueItems": true to the dependsOn array (to reject duplicates). Update the dependsOn definition (the "dependsOn" property in the schema) to include these constraints so validation will fail on empty or repeated dependency names.schema/openapi/health_gitProtocol.schema.json (2)
95-98: ConstraintransformDeleteStrategyto explicit values.The description implies a fixed set of allowed outcomes, but the schema currently accepts any string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_gitProtocol.schema.json` around lines 95 - 98, The transformDeleteStrategy property currently allows any string; constrain it to the explicit allowed outcomes by replacing the freeform "type": "string" with an enum listing the three valid values (e.g., "healthy", "unhealthy", "unchanged") and keep/update the description to match those enum values; target the transformDeleteStrategy schema entry to apply this change.
110-115: HardendependsOnvalidation for cleaner inputs.Consider preventing empty and duplicate dependency names.
🧩 Suggested schema tightening
"dependsOn": { "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "type": "array", + "uniqueItems": true, "description": "DependsOn lists the checks that must complete before this one runs" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_gitProtocol.schema.json` around lines 110 - 115, The dependsOn array currently allows empty and duplicate names; tighten the schema for the dependsOn property by adding "items": { "type": "string", "minLength": 1 } and set "uniqueItems": true on the dependsOn array (optionally add "minItems": 1 if an empty list should be rejected). Update the dependsOn definition so it enforces non-empty strings and rejects duplicates (referencing the dependsOn property in the schema).schema/openapi/health_junit.schema.json (1)
35-36: Near-duplicate description betweenCheckRelationshipdefinition andrelationshipspropertyThe object-level description on
CheckRelationship(Line 36) and the sibling description on therelationshipsproperty (Line 96) are nearly identical. The property-level description is useful for tools that surface per-field docs, but consider slightly differentiating the wording (e.g., make the property description a shorter one-liner and keep the full explanation on the definition) to avoid redundancy.Also applies to: 95-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/health_junit.schema.json` around lines 35 - 36, The CheckRelationship schema's object-level description and the relationships property description are nearly identical; edit the descriptions so the definition "CheckRelationship" keeps the full explanatory text and the sibling property "relationships" uses a concise one-line summary (or vice versa) to avoid duplication — locate the "CheckRelationship" definition and the "relationships" property in the schema and replace the property-level description with a short one-liner referencing linkage (e.g., "Links check results to components/configs via lookup expressions") while leaving the detailed explanation on the CheckRelationship definition.schema/openapi/scrape_plugin.schema.json (2)
83-85:ConfigProperties.valuetyped asinteger— may be too restrictive.If config property values can be floating-point numbers (e.g., percentages, rates),
"type": "integer"will reject them. Consider using"type": "number"to accept both integers and floats.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/scrape_plugin.schema.json` around lines 83 - 85, The schema for ConfigProperties currently declares "value" with "type": "integer", which disallows floats; change the JSON Schema for the ConfigProperties.value property to "type": "number" (or to ["integer","number"] if you want to explicitly accept both) so percentages/decimal rates are accepted; update the schema entry that defines "value" (the ConfigProperties.value definition) accordingly and run your schema validator/tests to ensure no downstream assumptions require strictly integer types.
368-388: Consider addingrequiredfields toScrapePlugin.Kubernetes-style resources typically require
kindandapiVersionat minimum. Withoutrequiredconstraints, an empty{}object would pass validation. If this schema is used to validate user-provided manifests, adding required fields would catch malformed input earlier.Example
"ScrapePlugin": { "properties": { "kind": { "type": "string" }, ... }, "additionalProperties": false, - "type": "object" + "type": "object", + "required": [ + "kind", + "apiVersion", + "metadata", + "spec" + ] },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema/openapi/scrape_plugin.schema.json` around lines 368 - 388, The ScrapePlugin schema currently allows an empty object to validate; add a required array to the ScrapePlugin definition listing at minimum "kind" and "apiVersion" so manifests missing those fields fail validation; update the ScrapePlugin object (the schema with "properties": { "kind", "apiVersion", "metadata", "spec", "status" }) to include "required": ["kind","apiVersion"] and keep additionalProperties set to false to enforce stricter validation.tests/http_connection_test.go (1)
107-108: Consider using Gomega'sHavePrefixmatcher for clearer assertions.
HavePrefixis more idiomatic and provides a better failure message thanBeTrue().♻️ Suggested diff
- Expect(strings.HasPrefix(authHeader, "AWS4-HMAC-SHA256")).To(BeTrue(), - "Authorization header should start with AWS4-HMAC-SHA256, got: "+authHeader) + Expect(authHeader).To(HavePrefix("AWS4-HMAC-SHA256"), + "Authorization header should start with AWS4-HMAC-SHA256")This also removes the need for the
"strings"import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/http_connection_test.go` around lines 107 - 108, Replace the current assertion Expect(strings.HasPrefix(authHeader, "AWS4-HMAC-SHA256")).To(BeTrue(), ...) with Gomega's idiomatic HavePrefix matcher by asserting Expect(authHeader).To(HavePrefix("AWS4-HMAC-SHA256")) (keeping the custom failure text if desired), and remove the now-unused "strings" import; update the test in tests/http_connection_test.go where authHeader is checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connection/http_test.go`:
- Around line 12-122: The test TestHTTPConnectionPretty uses testing's
t.Errorf/t.Fatalf; switch to Gomega by adding the import
"github.com/onsi/gomega" and creating g := gomega.NewWithT(t) at the start of
each subtest in TestHTTPConnectionPretty, then replace t.Errorf checks with
g.Expect(result).To(gomega.ContainSubstring(...)) and replace exclusion checks
with g.Expect(result).ToNot(gomega.ContainSubstring(...)); also convert any
t.Fatalf-style fatal assertions to appropriate g.Expect(...) assertions (e.g.,
NotTo(HaveOccurred()) or Equal/ContainSubstring) so all assertions use
gomega.NewWithT(t) and g.Expect.
In `@connection/http.go`:
- Around line 84-95: The Pretty() output currently appends raw query parameter
values via parsed.Query().Get(key) (inside the loop using
sortedQueryKeys(parsed.Query())), which leaks secrets; change Pretty() to
display only the parameter names and a redacted placeholder (e.g., "<redacted>"
or "***") instead of the real value, and make the identical change where
Hydrate() logs/uses that output (the same logic referenced at lines ~212-213).
Locate the loop that builds query parts (uses sortedQueryKeys and
parsed.Query().Get) and replace the value insertion with a constant redaction
string so keys are shown but values are never logged.
In `@connection/opensearch.go`:
- Around line 95-97: The merge currently only copies existing.InsecureSkipVerify
when it's true, which prevents an explicit inline false from overriding a
referenced true; make InsecureSkipVerify a pointer (e.g., *bool) on the
connection struct so you can tell "unset" vs explicit true/false, then update
the merge logic in connection/opensearch.go to set c.InsecureSkipVerify =
existing.InsecureSkipVerify only when c.InsecureSkipVerify is nil (i.e., not
explicitly provided); update any constructors/parsers and usages of
InsecureSkipVerify to handle the pointer semantics accordingly.
In `@connection/smb.go`:
- Around line 65-68: SMB domain is only read from conn.Properties["domain"] when
hydrating c.Domain, but SMBConnection.ToModel writes the domain into conn.URL,
so c.Domain can remain empty; update the hydration logic in the function that
sets c.Domain to also parse conn.URL (using the same URL format produced by
SMBConnection.ToModel) and extract the domain component when c.Domain == ""
(fall back to conn.Properties["domain"] only if URL does not contain the
domain). Ensure you reference and use the existing symbols c.Domain,
conn.Properties, and conn.URL so the extracted domain is assigned to c.Domain.
In `@schema/openapi/application.schema.json`:
- Around line 570-572: The ViewSection schema currently only requires "title",
allowing sections to omit both "viewRef" and "uiRef"; update the ViewSection
definition to require at least one of those two properties by adding an anyOf
(or oneOf) clause that mandates either {"required":["viewRef"]} or
{"required":["uiRef"]} alongside the existing "title" requirement so validation
fails when both are missing; target the ViewSection schema object and modify its
required/validation rules accordingly.
In `@schema/openapi/openapi.go`:
- Around line 19-21: The code uses gojsonschema (playbookSchemaLoader,
gojsonschema.NewReferenceLoaderFileSystem, gojsonschema.Validate) which doesn't
support Draft 2020-12; replace it with a 2020-12-capable validator (for example
github.com/santhosh-tekuri/jsonschema/v5 or github.com/qri-io/jsonschema), load
your file:// schema via that library's compiler (or AddResource/LoadURL using
http.FS(Schemas)), compile the schema and then validate the document bytes
against the compiled schema, and update imports and error handling accordingly
so validation honors 2020-12 rules.
In `@schema/openapi/scrape_plugin.schema.json`:
- Around line 434-438: The "Time" schema currently defined as an empty object
should be changed to an RFC3339 timestamp string: replace the object schema for
"Time" with a string type (e.g., "type": "string") and add the appropriate
format or pattern for RFC3339 (e.g., "format": "date-time" or an RFC3339 regex)
so fields like creationTimestamp/deletionTimestamp/ManagedFieldsEntry.time
validate correctly; update the "Time" definition in
schema/openapi/scrape_plugin.schema.json (the "Time" schema entry) to use
string+format and remove the empty object/additionalProperties constraints.
---
Outside diff comments:
In `@connection/http.go`:
- Around line 352-360: The current RoundTrip mutates rt.Base by calling rt.Base
= oauthTransport.RoundTripper(rt.Base), which is unsafe because RoundTrip can
run concurrently and will repeatedly stack middleware; instead, do not assign
back to rt.Base—in RoundTrip capture the current base transport into a local
variable (e.g., base := rt.Base), build the wrapped transport via
oauthTransport.RoundTripper(base), and use that local wrapped transport to
perform the request; apply the same pattern for other middleware-wrapping
instances (e.g., the other oauthTransport creation) so no shared rt.Base is ever
mutated inside RoundTrip.
In `@schema/openapi/config_logs.schema.json`:
- Around line 497-524: OpenSearchConfig currently allows neither endpoint field
to be present; update the JSON schema so at least one of the endpoint sources is
required by adding an anyOf (or oneOf) clause that enforces either "connection"
or "address" is present (e.g., anyOf:
[{required:["connection"]},{required:["address"]}). Modify the schema that
defines the OpenSearchConfig properties (fields "connection" and "address" and
the surrounding object schema) to include this anyOf/oneOf constraint while
keeping additionalProperties:false and the existing required list for "index"
and "query".
---
Minor comments:
In `@connection/loki.go`:
- Around line 31-36: The current hydration code unconditionally sets c.Username
and c.Password to &types.EnvVar{ValueStatic: conn.Username/conn.Password} even
when conn.Username or conn.Password are empty, creating non-nil empty env var
objects; change the logic in the hydration path (where c.Username and c.Password
are set) to only assign a new types.EnvVar when conn.Username/conn.Password are
non-empty (e.g., check conn.Username != "" and conn.Password != "") so you don't
materialize empty pointers and alter downstream JSON/behavior.
In `@connection/merge_test.go`:
- Around line 95-101: The test sets additional merge/override/fallback values
earlier in the test but never asserts their outcomes; after calling
conn.Populate(ctx) add gomega assertions mirroring the existing pattern (e.g.,
g.Expect(conn.<FieldName>).To(gomega.Equal("<expected-value>")) or
g.Expect(conn.<NestedStruct>.ValueStatic).To(gomega.Equal("<expected-value>")))
for each configured merge field that was set in this test (the same fields
configured at the earlier setup sites referenced in the comment), and add the
same corresponding assertions in the other two test blocks that set those values
(the blocks around the other mentioned setup locations) so each configured
override/fallback field is verified.
In `@context/envvar.go`:
- Around line 60-61: The log gating and emission verbosity are mismatched: the
condition uses ctx.Logger.IsLevelEnabled(2) but the message is emitted with
ctx.Logger.V(5).Infof, so align them by changing the gate to
ctx.Logger.IsLevelEnabled(5) (or alternatively change V(5) to V(2)); update the
condition that currently reads ctx.Logger.IsLevelEnabled(2) &&
properties.On(false, "envvar.lookup.log") so it uses
ctx.Logger.IsLevelEnabled(5) && properties.On(false, "envvar.lookup.log") to
ensure the Infof call in ctx.Logger.V(5).Infof actually executes when expected.
In `@schema/openapi/health_azuredevops.schema.json`:
- Around line 92-94: The description for the property "thresholdMillis"
currently says "(Optional)" but the JSON Schema still lists "thresholdMillis" in
the schema's required array; update the schema so documentation and contract
match by either removing "thresholdMillis" from the schema's required list or
changing the property description to remove "(Optional)". Locate the
"thresholdMillis" property in the schema and the surrounding "required" array
for the same object (referencing the property name "thresholdMillis" and the
schema's "required" array) and make the choice consistent across both places.
In `@schema/openapi/health_databasebackupcheck.schema.json`:
- Around line 50-53: The "namespace" property description is incomplete (it ends
with "e.g."); update the "namespace" field description in the schema to a
complete sentence or remove the trailing "e.g." — for example: "Namespace to
insert the check into, if different from the namespace the canary is defined in
(e.g., 'prod' or 'staging')." Locate and edit the "namespace" property in the
health_databasebackupcheck JSON schema to replace the unfinished text with the
chosen complete wording.
In `@schema/openapi/health_dockerPull.schema.json`:
- Around line 62-65: The "namespace" property description in
schema/openapi/health_dockerPull.schema.json is truncated ("..., e.g.") — update
the description for the "namespace" field to be a complete sentence and include
the intended example or clause (e.g., "Namespace to insert the check into, if
different from the namespace the canary is defined in, e.g. 'prod' or
'staging'") or, if the schema is generated, fix the source Go struct doc comment
for the Namespace field so the generator emits the full description; ensure the
description string for "namespace" is clear and no longer ends with an
unfinished "e.g.".
In `@schema/openapi/health_folder.schema.json`:
- Around line 135-137: Update the JSON schema field descriptions: fix the typo
in the "path" description by changing "/path/tp/folder" to "/path/to/folder" and
ensure examples still show S3/GCS and local paths; and correct the "maxCount"
description (referenced as property "maxCount") so it describes the maximum
number of items (not minimum/MinCount) and mentions it is an upper limit. Locate
and update the "path" and "maxCount" property descriptions in the schema object
to reflect these exact wording fixes.
In `@schema/openapi/health_gitProtocol.schema.json`:
- Around line 84-86: The "namespace" schema description in
schema/openapi/health_gitProtocol.schema.json is incomplete; update the
"description" value for the namespace property to be a full sentence and include
a concrete example (for instance: "Namespace to insert the check into, if
different from the namespace the canary is defined in (e.g., 'team-a' or
'default')."). Locate the "namespace" property in the schema and replace the
existing truncated text with the completed sentence and example.
In `@schema/openapi/health_helm.schema.json`:
- Around line 96-98: The "namespace" property description for the schema
property named "namespace" is incomplete (ends with "e.g."); update the
description string in health_helm.schema.json for the "namespace" field to be a
complete sentence and include a short example (for instance: "Namespace to
insert the check into, if different from the namespace the canary is defined in,
e.g. 'monitoring'"). Ensure punctuation and grammar are fixed ("from" instead of
"to" where appropriate) and keep the wording concise for generated docs.
In `@schema/openapi/health_junit.schema.json`:
- Around line 49-50: The "namespace" property description in
health_junit.schema.json is truncated ("e.g.") — update the "description" for
the namespace property to complete the sentence and include an example (for
example: "Namespace to insert the check into, if different to the namespace the
canary is defined, e.g. 'monitoring' or 'default'."), ensuring the text remains
a single string value for the "namespace" schema entry so consumers see a clear
explanation and example.
- Around line 57-58: The schema uses sibling descriptions alongside "$ref" for
the properties labels and relationships (declared under "$defs" in this JSON
Schema); to maintain compatibility with tools that strip sibling keywords,
replace the direct "$ref" sibling pattern for the labels and relationships
property definitions with an allOf wrapper that contains the $ref entry and a
separate schema object holding the description (i.e., convert the labels and
relationships property definitions to use allOf with one element referencing
"#/$defs/Labels" or the appropriate def and another element giving the
description), ensuring the schema still targets Draft 2020-12 but avoids tooling
that removes sibling keywords.
In `@schema/openapi/health_mongodb.schema.json`:
- Around line 127-130: The "namespace" property's description in the OpenAPI
JSON is truncated and ends with an unfinished "e.g."; update the description for
the "namespace" field (property name: namespace) to either remove the trailing
"e.g." or complete it with a concrete example—e.g., "Namespace to insert the
check into, if different from the namespace the canary is defined in, e.g.
'monitoring'."—so the sentence reads as a full, clear description.
In `@schema/openapi/health_mysql.schema.json`:
- Around line 162-164: Update the incomplete schema description for the
"namespace" property (the description string that currently ends with "e.g.") to
include a concrete example (e.g. "e.g. 'monitoring' or 'default'") and make the
description a complete sentence; also fix the grammar typo in the other
description referenced around lines 229-230 by editing that property's
"description" field to read clearly (for example: "Namespace to insert the check
into, if different from the namespace the canary is defined in. e.g.
'monitoring'"). Ensure you only modify the JSON "description" values for the
relevant schema properties.
In `@schema/openapi/health_pubsub.schema.json`:
- Around line 181-183: The "namespace" property description is truncated ("e.g."
with no example); update the description for the namespace property in the
schema to either remove the trailing "e.g." or complete the example (for
example: "Namespace to insert the check into, if different to the namespace the
canary is defined, e.g. monitoring"). Edit the description string for the
"namespace" property so it reads as a complete sentence and includes a concrete
example if you choose to keep "e.g.".
In `@schema/openapi/health_restic.schema.json`:
- Around line 241-243: Update the restic repository description strings in
schema/openapi/health_restic.schema.json to remove credential-like examples and
fix the typo: replace occurrences of "user:pass" with a neutral placeholder such
as "<username>:<password>" (or "{username}:{password}") and correct "reposiotry"
to "repository"; apply the same wording changes to the other occurrence
mentioned (lines ~253-255) so both descriptions use the cleaned placeholder and
the corrected spelling.
In `@schema/openapi/scrape_plugin.schema.json`:
- Around line 114-118: The FieldsV1 schema currently sets
"additionalProperties": false which forbids arbitrary managed-fields content;
update the FieldsV1 definition so it accepts an opaque JSON blob by allowing
arbitrary properties (e.g., remove or change "additionalProperties": false to
"additionalProperties": true or replace the properties block with a free-form
object schema), keeping "type": "object" so real Kubernetes managedFields data
won't be rejected; locate the FieldsV1 schema entry and make this change.
---
Nitpick comments:
In `@schema/openapi/health_databasebackupcheck.schema.json`:
- Around line 77-83: The dependsOn array allows empty or duplicate strings;
tighten the JSON Schema for the dependsOn property by adding "minLength": 1 to
its items (so empty strings are invalid) and add "uniqueItems": true to the
dependsOn array (to reject duplicates). Update the dependsOn definition (the
"dependsOn" property in the schema) to include these constraints so validation
will fail on empty or repeated dependency names.
In `@schema/openapi/health_dynatrace.schema.json`:
- Around line 76-81: Tighten the dependsOn array schema by rejecting empty
strings and duplicates: in the dependsOn definition (the "dependsOn" property
where "items" currently has "type":"string"), add constraints on the items such
as a non-empty string requirement (e.g., "minLength": 1) and on the array itself
enable deduplication validation (e.g., "uniqueItems": true) so empty entries and
duplicate check names are disallowed.
In `@schema/openapi/health_gitProtocol.schema.json`:
- Around line 95-98: The transformDeleteStrategy property currently allows any
string; constrain it to the explicit allowed outcomes by replacing the freeform
"type": "string" with an enum listing the three valid values (e.g., "healthy",
"unhealthy", "unchanged") and keep/update the description to match those enum
values; target the transformDeleteStrategy schema entry to apply this change.
- Around line 110-115: The dependsOn array currently allows empty and duplicate
names; tighten the schema for the dependsOn property by adding "items": {
"type": "string", "minLength": 1 } and set "uniqueItems": true on the dependsOn
array (optionally add "minItems": 1 if an empty list should be rejected). Update
the dependsOn definition so it enforces non-empty strings and rejects duplicates
(referencing the dependsOn property in the schema).
In `@schema/openapi/health_junit.schema.json`:
- Around line 35-36: The CheckRelationship schema's object-level description and
the relationships property description are nearly identical; edit the
descriptions so the definition "CheckRelationship" keeps the full explanatory
text and the sibling property "relationships" uses a concise one-line summary
(or vice versa) to avoid duplication — locate the "CheckRelationship" definition
and the "relationships" property in the schema and replace the property-level
description with a short one-liner referencing linkage (e.g., "Links check
results to components/configs via lookup expressions") while leaving the
detailed explanation on the CheckRelationship definition.
In `@schema/openapi/scrape_plugin.schema.json`:
- Around line 83-85: The schema for ConfigProperties currently declares "value"
with "type": "integer", which disallows floats; change the JSON Schema for the
ConfigProperties.value property to "type": "number" (or to ["integer","number"]
if you want to explicitly accept both) so percentages/decimal rates are
accepted; update the schema entry that defines "value" (the
ConfigProperties.value definition) accordingly and run your schema
validator/tests to ensure no downstream assumptions require strictly integer
types.
- Around line 368-388: The ScrapePlugin schema currently allows an empty object
to validate; add a required array to the ScrapePlugin definition listing at
minimum "kind" and "apiVersion" so manifests missing those fields fail
validation; update the ScrapePlugin object (the schema with "properties": {
"kind", "apiVersion", "metadata", "spec", "status" }) to include "required":
["kind","apiVersion"] and keep additionalProperties set to false to enforce
stricter validation.
In `@tests/http_connection_test.go`:
- Around line 107-108: Replace the current assertion
Expect(strings.HasPrefix(authHeader, "AWS4-HMAC-SHA256")).To(BeTrue(), ...) with
Gomega's idiomatic HavePrefix matcher by asserting
Expect(authHeader).To(HavePrefix("AWS4-HMAC-SHA256")) (keeping the custom
failure text if desired), and remove the now-unused "strings" import; update the
test in tests/http_connection_test.go where authHeader is checked.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (97)
.gitignoreconnection/aws.goconnection/azure.goconnection/gcp.goconnection/gcs.goconnection/git.goconnection/http.goconnection/http_test.goconnection/kubernetes.goconnection/loki.goconnection/merge_test.goconnection/opensearch.goconnection/s3.goconnection/sftp.goconnection/smb.goconnection/sql.goconnection/zz_generated.deepcopy.gocontext/envvar.gogo.modpostgrest/postgrest.goschema/openapi/application.schema.jsonschema/openapi/canary.schema.jsonschema/openapi/canary.spec.schema.jsonschema/openapi/component.schema.jsonschema/openapi/component.spec.schema.jsonschema/openapi/config_aws.schema.jsonschema/openapi/config_azure.schema.jsonschema/openapi/config_azuredevops.schema.jsonschema/openapi/config_exec.schema.jsonschema/openapi/config_file.schema.jsonschema/openapi/config_gcp.schema.jsonschema/openapi/config_githubactions.schema.jsonschema/openapi/config_http.schema.jsonschema/openapi/config_kubernetes.schema.jsonschema/openapi/config_kubernetesfile.schema.jsonschema/openapi/config_logs.schema.jsonschema/openapi/config_slack.schema.jsonschema/openapi/config_sql.schema.jsonschema/openapi/config_terraform.schema.jsonschema/openapi/config_trivy.schema.jsonschema/openapi/connection.definitions.jsonschema/openapi/connection.schema.jsonschema/openapi/health_alertmanager.schema.jsonschema/openapi/health_awsconfig.schema.jsonschema/openapi/health_awsconfigrule.schema.jsonschema/openapi/health_azuredevops.schema.jsonschema/openapi/health_catalog.schema.jsonschema/openapi/health_cloudwatch.schema.jsonschema/openapi/health_containerdPull.schema.jsonschema/openapi/health_containerdPush.schema.jsonschema/openapi/health_databasebackupcheck.schema.jsonschema/openapi/health_dns.schema.jsonschema/openapi/health_dockerPull.schema.jsonschema/openapi/health_dockerPush.schema.jsonschema/openapi/health_dynatrace.schema.jsonschema/openapi/health_elasticsearch.schema.jsonschema/openapi/health_exec.schema.jsonschema/openapi/health_folder.schema.jsonschema/openapi/health_gitProtocol.schema.jsonschema/openapi/health_github.schema.jsonschema/openapi/health_helm.schema.jsonschema/openapi/health_http.schema.jsonschema/openapi/health_icmp.schema.jsonschema/openapi/health_jmeter.schema.jsonschema/openapi/health_junit.schema.jsonschema/openapi/health_kubernetes.schema.jsonschema/openapi/health_ldap.schema.jsonschema/openapi/health_mongodb.schema.jsonschema/openapi/health_mssql.schema.jsonschema/openapi/health_mysql.schema.jsonschema/openapi/health_namespace.schema.jsonschema/openapi/health_opensearch.schema.jsonschema/openapi/health_pod.schema.jsonschema/openapi/health_postgres.schema.jsonschema/openapi/health_prometheus.schema.jsonschema/openapi/health_pubsub.schema.jsonschema/openapi/health_redis.schema.jsonschema/openapi/health_restic.schema.jsonschema/openapi/health_s3.schema.jsonschema/openapi/health_tcp.schema.jsonschema/openapi/health_webhook.schema.jsonschema/openapi/notification.definitions.jsonschema/openapi/notification.schema.jsonschema/openapi/openapi.goschema/openapi/playbook-spec.schema.jsonschema/openapi/playbook.definitions.jsonschema/openapi/playbook.schema.jsonschema/openapi/scrape_config.definitions.jsonschema/openapi/scrape_config.schema.jsonschema/openapi/scrape_config.spec.schema.jsonschema/openapi/scrape_plugin.schema.jsonschema/openapi/scrape_plugin_spec.schema.jsonschema/openapi/topology.definitions.jsonschema/openapi/topology.schema.jsonschema/openapi/topology.spec.schema.jsonschema/openapi/view.schema.jsontests/http_connection_test.go
Comment on lines
+84
to
+95
| if t.URL != "" { | ||
| s = s.NewLine() | ||
| if parsed, err := netURL.Parse(t.URL); err == nil && parsed.Host != "" { | ||
| s = s.AddText(parsed.Scheme+"://"+parsed.Host, "font-bold text-blue-600"). | ||
| AddText(parsed.Path, "text-gray-500") | ||
| for i, key := range sortedQueryKeys(parsed.Query()) { | ||
| prefix := "?" | ||
| if i > 0 { | ||
| prefix = "&" | ||
| } | ||
| s = s.NewLine().AddText(fmt.Sprintf(" %s%s=%s", prefix, key, parsed.Query().Get(key)), "text-gray-400") | ||
| } |
Contributor
There was a problem hiding this comment.
Redact query parameter values before logging connection previews.
Pretty() currently includes raw query values, and Hydrate() logs that output. This can expose credentials/tokens in logs.
🔧 Proposed fix
- s = s.NewLine().AddText(fmt.Sprintf(" %s%s=%s", prefix, key, parsed.Query().Get(key)), "text-gray-400")
+ s = s.NewLine().AddText(fmt.Sprintf(" %s%s=****", prefix, key), "text-gray-400")Also applies to: 212-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connection/http.go` around lines 84 - 95, The Pretty() output currently
appends raw query parameter values via parsed.Query().Get(key) (inside the loop
using sortedQueryKeys(parsed.Query())), which leaks secrets; change Pretty() to
display only the parameter names and a redacted placeholder (e.g., "<redacted>"
or "***") instead of the real value, and make the identical change where
Hydrate() logs/uses that output (the same logic referenced at lines ~212-213).
Locate the loop that builds query parts (uses sortedQueryKeys and
parsed.Query().Get) and replace the value insertion with a constant redaction
string so keys are shown but values are never logged.
Comment on lines
+95
to
+97
| if existing.InsecureSkipVerify { | ||
| c.InsecureSkipVerify = existing.InsecureSkipVerify | ||
| } |
Contributor
There was a problem hiding this comment.
InsecureSkipVerify merge is one-way and can silently keep insecure TLS on.
Current logic only preserves the inline value when it is true. An explicit inline false cannot override a hydrated true, so callers cannot force TLS verification once a referenced connection has insecure mode enabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connection/opensearch.go` around lines 95 - 97, The merge currently only
copies existing.InsecureSkipVerify when it's true, which prevents an explicit
inline false from overriding a referenced true; make InsecureSkipVerify a
pointer (e.g., *bool) on the connection struct so you can tell "unset" vs
explicit true/false, then update the merge logic in connection/opensearch.go to
set c.InsecureSkipVerify = existing.InsecureSkipVerify only when
c.InsecureSkipVerify is nil (i.e., not explicitly provided); update any
constructors/parsers and usages of InsecureSkipVerify to handle the pointer
semantics accordingly.
Comment on lines
+65
to
+68
| if c.Domain == "" { | ||
| if domain, ok := conn.Properties["domain"]; ok { | ||
| c.Domain = domain | ||
| } |
Contributor
There was a problem hiding this comment.
Populate SMB domain from conn.URL (not only conn.Properties["domain"]).
At Line [65], domain is hydrated only from conn.Properties["domain"], but SMBConnection.ToModel writes domain to URL at Line [35]. This can leave c.Domain empty for valid referenced SMB connections.
💡 Proposed fix
- if c.Domain == "" {
- if domain, ok := conn.Properties["domain"]; ok {
- c.Domain = domain
- }
- }
+ if c.Domain == "" {
+ if conn.URL != "" {
+ c.Domain = conn.URL
+ } else if domain, ok := conn.Properties["domain"]; ok {
+ c.Domain = domain
+ }
+ }📝 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.
Suggested change
| if c.Domain == "" { | |
| if domain, ok := conn.Properties["domain"]; ok { | |
| c.Domain = domain | |
| } | |
| if c.Domain == "" { | |
| if conn.URL != "" { | |
| c.Domain = conn.URL | |
| } else if domain, ok := conn.Properties["domain"]; ok { | |
| c.Domain = domain | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connection/smb.go` around lines 65 - 68, SMB domain is only read from
conn.Properties["domain"] when hydrating c.Domain, but SMBConnection.ToModel
writes the domain into conn.URL, so c.Domain can remain empty; update the
hydration logic in the function that sets c.Domain to also parse conn.URL (using
the same URL format produced by SMBConnection.ToModel) and extract the domain
component when c.Domain == "" (fall back to conn.Properties["domain"] only if
URL does not contain the domain). Ensure you reference and use the existing
symbols c.Domain, conn.Properties, and conn.URL so the extracted domain is
assigned to c.Domain.
Comment on lines
570
to
572
| "required": [ | ||
| "title", | ||
| "viewRef" | ||
| "title" | ||
| ] |
Contributor
There was a problem hiding this comment.
Require at least one section target (viewRef or uiRef).
Line 571 now only requires title, so ViewSection can validate with neither viewRef nor uiRef, which creates structurally valid but non-renderable sections.
🔧 Suggested schema fix
"ViewSection": {
"properties": {
"title": {
"type": "string"
},
"icon": {
"type": "string"
},
"viewRef": {
"$ref": "#/$defs/ViewRef"
},
"uiRef": {
"$ref": "#/$defs/UIRef"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"title"
- ]
+ ],
+ "anyOf": [
+ { "required": ["viewRef"] },
+ { "required": ["uiRef"] }
+ ]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema/openapi/application.schema.json` around lines 570 - 572, The
ViewSection schema currently only requires "title", allowing sections to omit
both "viewRef" and "uiRef"; update the ViewSection definition to require at
least one of those two properties by adding an anyOf (or oneOf) clause that
mandates either {"required":["viewRef"]} or {"required":["uiRef"]} alongside the
existing "title" requirement so validation fails when both are missing; target
the ViewSection schema object and modify its required/validation rules
accordingly.
Comment on lines
+19
to
+21
| var playbookSchemaLoader = gojsonschema.NewReferenceLoaderFileSystem("file:///"+path, http.FS(Schemas)) | ||
| documentLoader := gojsonschema.NewBytesLoader(schema) | ||
| result, err := gojsonschema.Validate(playbookSchemaLoader, documentLoader) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking schema files for draft declarations ==="
find schema/openapi -name "*.json" -type f -exec grep -l '\$schema' {} \;
echo -e "\n=== Schema draft declarations ==="
find schema/openapi -name "*.json" -type f -exec grep '\$schema' {} + | head -20
echo -e "\n=== gojsonschema in go.mod ==="
grep -A2 -B2 'gojsonschema' go.mod 2>/dev/null || echo "Not found in go.mod"
echo -e "\n=== gojsonschema in go.sum ==="
grep 'gojsonschema' go.sum 2>/dev/null | head -5 || echo "Not found in go.sum"Repository: flanksource/duty
Length of output: 5586
🌐 Web query:
gojsonschema github.com/xeipuuv/gojsonschema JSON Schema draft support versions
💡 Result:
github.com/xeipuuv/gojsonschema supports JSON Schema Draft-04, Draft-06, and Draft-07. [1][2]
- By default it auto-detects which of those drafts to use based on the schema’s
$schemavalue; if"$schema"is missing (or not explicit), it uses a hybrid mode that merges features across those drafts. [1][2] - You can force a specific draft by setting
SchemaLoader.Draft(e.g.,gojsonschema.Draft7) and disabling auto-detection viaSchemaLoader.AutoDetect = false. [1][2] - It does not claim support for newer “modern” drafts like 2019-09 or 2020-12. [1][2]
Sources: [1] [2]
Replace gojsonschema with a JSON Schema 2020-12 capable validator.
Your schemas declare draft 2020-12, but github.com/xeipuuv/gojsonschema only supports Draft-04, Draft-06, and Draft-07. This mismatch causes validation to silently miss 2020-12 constraints, producing incorrect outcomes. Switch to a validator with 2020-12 support or downgrade schema drafts to Draft-07.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema/openapi/openapi.go` around lines 19 - 21, The code uses gojsonschema
(playbookSchemaLoader, gojsonschema.NewReferenceLoaderFileSystem,
gojsonschema.Validate) which doesn't support Draft 2020-12; replace it with a
2020-12-capable validator (for example github.com/santhosh-tekuri/jsonschema/v5
or github.com/qri-io/jsonschema), load your file:// schema via that library's
compiler (or AddResource/LoadURL using http.FS(Schemas)), compile the schema and
then validate the document bytes against the compiled schema, and update imports
and error handling accordingly so validation honors 2020-12 rules.
Comment on lines
+434
to
+438
| "Time": { | ||
| "properties": {}, | ||
| "additionalProperties": false, | ||
| "type": "object" | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the Time definition at lines 434-438
sed -n '434,438p' schema/openapi/scrape_plugin.schema.jsonRepository: flanksource/duty
Length of output: 164
🏁 Script executed:
# Check the full context around Time to understand its definition
sed -n '430,445p' schema/openapi/scrape_plugin.schema.jsonRepository: flanksource/duty
Length of output: 412
🏁 Script executed:
# Search for all references to Time in the schema file
rg 'Time' schema/openapi/scrape_plugin.schema.json -nRepository: flanksource/duty
Length of output: 258
🏁 Script executed:
# Check how Time is referenced in ManagedFieldsEntry and ObjectMeta definitions
sed -n '182,220p' schema/openapi/scrape_plugin.schema.json | head -50Repository: flanksource/duty
Length of output: 868
🏁 Script executed:
# Look for other JSON schema files in the schema/openapi directory
fd --extension json . schema/openapi/ 2>/dev/null | head -20Repository: flanksource/duty
Length of output: 872
🏁 Script executed:
# Check if there are other schema files that define Time
fd --extension json . schema/openapi/ -exec grep -l '"Time"' {} \;Repository: flanksource/duty
Length of output: 8198
🏁 Script executed:
# Check how Time is defined in another schema file (e.g., canary.schema.json)
jq '.[$defs].Time // empty' schema/openapi/canary.schema.json 2>/dev/nullRepository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Check another schema file to see the pattern
jq '.[$defs].Time // empty' schema/openapi/component.schema.json 2>/dev/nullRepository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Let's search for all Time definitions across schema files to see if there's a pattern
for file in schema/openapi/*.schema.json; do
timeDef=$(jq '.[$defs].Time // empty' "$file" 2>/dev/null)
if [ ! -z "$timeDef" ]; then
echo "=== $(basename $file) ==="
echo "$timeDef" | head -10
fi
doneRepository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Try with different jq syntax - $defs might not exist in all files
grep -A5 '"Time"' schema/openapi/canary.schema.json | head -20Repository: flanksource/duty
Length of output: 186
🏁 Script executed:
# Check component.schema.json
grep -A5 '"Time"' schema/openapi/component.schema.json | head -20Repository: flanksource/duty
Length of output: 186
🏁 Script executed:
# Check multiple schema files to see Time patterns
for file in schema/openapi/canary.schema.json schema/openapi/component.schema.json schema/openapi/application.schema.json; do
echo "=== $(basename $file) ==="
grep -A5 '"Time"' "$file" | head -10
echo ""
doneRepository: flanksource/duty
Length of output: 525
🏁 Script executed:
# Verify schema validation behavior with an example
# Check if there are any test files or actual usage of these schemas
find . -type f -name "*.go" -o -name "*.py" | xargs grep -l "scrape_plugin.schema.json\|Time.*RFC\|metav1.Time" 2>/dev/null | head -10Repository: flanksource/duty
Length of output: 139
🏁 Script executed:
# Check if these schemas are actually used for validation
rg "schema.*json.*Time|validation.*Time" -t go -t py | head -20Repository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Let's check if there's any code that shows how Time values are actually handled
rg "creationTimestamp|deletionTimestamp" -A2 -B2 | head -30Repository: flanksource/duty
Length of output: 2063
🏁 Script executed:
# Count how many schema files have this exact Time definition
grep -r '"Time": {' schema/openapi/ | grep -c '"properties": {}' && echo "---" && grep -r '"Time": {' schema/openapi/ | wc -lRepository: flanksource/duty
Length of output: 61
🏁 Script executed:
# Check if these schemas are actually used for validation in the codebase
rg "scrape_plugin.schema.json|canary.schema.json|component.schema.json" --type go | head -20Repository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Look for how these schema files are loaded and used
rg "json-schema|validate.*schema|ajv\|jsonschema" -t go | head -20Repository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Check if there's any schema validation being done at all
find . -type f -name "*.go" -exec grep -l "schema.*valid\|json.*schema.*validat" {} \; 2>/dev/nullRepository: flanksource/duty
Length of output: 83
🏁 Script executed:
# Let's see if ManagedFieldsEntry.time is actually being populated with values anywhere
rg "time.*:" tests/fixtures/dummy/ | head -10Repository: flanksource/duty
Length of output: 290
🏁 Script executed:
# Check the source of these schema files - are they generated or manually maintained?
head -20 schema/openapi/scrape_plugin.schema.jsonRepository: flanksource/duty
Length of output: 541
🏁 Script executed:
# Verify the schema is actually being used for validation
grep -r "scrape_plugin.schema.json\|\.schema\.json" . --include="*.go" | grep -v "Binary" | head -20Repository: flanksource/duty
Length of output: 240
🏁 Script executed:
# Check kubernetes/dynamic.go to see how schemas are used
cat kubernetes/dynamic.go | head -100Repository: flanksource/duty
Length of output: 2614
🏁 Script executed:
# Check the generate-schemas tool to see if Time definitions are generated or hardcoded
cat hack/generate-schemas/main.go | head -150Repository: flanksource/duty
Length of output: 853
🏁 Script executed:
# Check if there's a template or source that defines Time
grep -r "Time" hack/generate-schemas/ --include="*.go"Repository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Look at the openapi validation to understand how schemas are used
cat schema/openapi/openapi.goRepository: flanksource/duty
Length of output: 795
🏁 Script executed:
# Check if ValidateSpec is actually called for scrape_plugin schemas
rg "scrape_plugin|ValidateSpec" -t go | grep -v "Binary"Repository: flanksource/duty
Length of output: 324
🏁 Script executed:
# Check the git history to see if these schema files were recently added
git log --oneline -- "schema/openapi/scrape_plugin.schema.json" | head -5Repository: flanksource/duty
Length of output: 84
🏁 Script executed:
# Look for where Time might come from in the code generation
rg "metav1.Time|Time.*struct" -t go | head -20Repository: flanksource/duty
Length of output: 221
🏁 Script executed:
# Verify if scrape_plugin schema validation is actually being enforced
rg "ValidateSpec.*scrape" -t goRepository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Check if there's a broader schema validation pattern
rg "gojsonschema\|Validate" -t go | grep -v "Binary" | head -15Repository: flanksource/duty
Length of output: 42
🏁 Script executed:
# Examine actual usage: are scrape_plugin resources being validated?
rg "ScrapePlugin" -t go | head -10Repository: flanksource/duty
Length of output: 180
Time definition should be a string, not an empty object.
Kubernetes metav1.Time serializes as an RFC3339 string (e.g., "2025-05-30T18:37:01Z"). Defining it as an empty object with additionalProperties: false means any document with actual timestamp values in creationTimestamp, deletionTimestamp, or ManagedFieldsEntry.time will fail schema validation.
Proposed fix
"Time": {
- "properties": {},
- "additionalProperties": false,
- "type": "object"
+ "type": "string",
+ "format": "date-time"
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema/openapi/scrape_plugin.schema.json` around lines 434 - 438, The "Time"
schema currently defined as an empty object should be changed to an RFC3339
timestamp string: replace the object schema for "Time" with a string type (e.g.,
"type": "string") and add the appropriate format or pattern for RFC3339 (e.g.,
"format": "date-time" or an RFC3339 regex) so fields like
creationTimestamp/deletionTimestamp/ManagedFieldsEntry.time validate correctly;
update the "Time" definition in schema/openapi/scrape_plugin.schema.json (the
"Time" schema entry) to use string+format and remove the empty
object/additionalProperties constraints.
- Add Pretty()/String() with clicky text API for HTTPConnection - Add .gitignore entries for out.html, specs/*.md, generate-openapi - Reduce envvar lookup log verbosity with properties gate
Inline values specified on a connection struct now take precedence over values loaded from the connection model during hydration. Previously, hydration unconditionally overwrote all fields.
- Add AWSSigV4 struct with SigV4 request signing in round tripper - Support ConnectionTypeAWS and ConnectionTypeAzure in NewHTTPConnection - Preserve inline fields when hydrating HTTP connections - Move Hydrate call after connection type switch for all types
moshloop
force-pushed
the
pr/connections
branch
from
a083791 to
d9340a3
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
dependsOnpropertyImprovements
Documentation