Add Datadog write operations for workflow builder#45
Merged
Conversation
Extend the Datadog integration to support write operations (create
monitor, send event, mute monitor) in addition to the existing
read-only queries, for use by the workflow builder's Datadog actions.
Proto (message.proto):
- Add DATADOG_CREATE_MONITOR (6), DATADOG_SEND_EVENT (7),
DATADOG_MUTE_MONITOR (8) to DatadogQueryType enum
- Add json_body field (10) to DatadogQueryRequest for write payloads
- Regenerate Go bindings
Operator (datadog_executor.go):
- Add createMonitor: POST /api/v1/monitor with json_body
- Add sendEvent: POST /api/v1/events with json_body (API key only)
- Add muteMonitor: POST /api/v1/monitor/{id}/mute using filter as ID
- Wire new cases into ExecuteQuery switch
Made-with: Cursor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtends protobufs to add Datadog write/control and ArgoCD query types; adds Datadog write handlers; introduces an ArgoCD executor (discovery, auth, list/get/sync); updates StreamClient to route Datadog/ArgoCD on control and data streams with startup probe and an 8MB data-stream response cap. Changes
Sequence Diagram(s)sequenceDiagram
participant Server
participant StreamClient as Client
participant DatadogExec as DatadogExecutor
participant ArgoCDExec as ArgoCDExecutor
participant Kube as Kubernetes
participant DatadogAPI as Datadog
participant ArgoCDAPI as ArgoCD
Server->>StreamClient: StreamDataResponse (DatadogQueryRequest)
alt Datadog query (QueryType < 10)
StreamClient->>DatadogExec: ExecuteQuery(ddRequest)
DatadogExec->>DatadogAPI: GET/POST /api/v1/... (json_body/filter)
DatadogAPI-->>DatadogExec: HTTP response
DatadogExec-->>StreamClient: DatadogQueryResponse
else ArgoCD query (QueryType >= 10)
StreamClient->>ArgoCDExec: ExecuteQuery(ddRequest)
ArgoCDExec->>Kube: discover argocd-server service / read admin Secret
ArgoCDExec->>ArgoCDAPI: POST /api/v1/session (login)
ArgoCDAPI-->>ArgoCDExec: auth token
ArgoCDExec->>ArgoCDAPI: GET/POST /api/v1/applications... (filtered/json_body)
ArgoCDAPI-->>ArgoCDExec: HTTP response
ArgoCDExec-->>StreamClient: DatadogQueryResponse
end
alt data-stream response > 8MB
StreamClient->>StreamClient: truncate/clear payload, set 413
end
StreamClient->>Server: StreamDataRequest (DatadogQueryResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/cloud/v1/message.proto`:
- Around line 1322-1331: The enum DatadogQueryType currently has values like
DATADOG_QUERY_METRICS that violate Buf's ENUM_VALUE_PREFIX rule because they
don't include the enum name prefix; rename each enum value to include the enum
name prefix (e.g. change DATADOG_QUERY_METRICS -> DATADOG_QUERY_TYPE_METRICS,
DATADOG_QUERY_EVENTS -> DATADOG_QUERY_TYPE_EVENTS, DATADOG_QUERY_UNSPECIFIED ->
DATADOG_QUERY_TYPE_UNSPECIFIED, etc.), update any service/handler code and tests
that reference these symbols to the new names, and regenerate compiled protobuf
artifacts so the codebase and linting are consistent with the DatadogQueryType
symbol changes.
In `@pkg/datadog_executor/datadog_executor.go`:
- Around line 123-126: The DATADOG_SEND_EVENT branch is currently blocked by the
global app-key gate, making sendEvent unreachable even though sendEvent calls
doPost(..., false) and only needs the API key; update the early app-key presence
check to allow v1.DatadogQueryType_DATADOG_SEND_EVENT to bypass the app-key
requirement (i.e., only enforce the app-key gate for query types that actually
need it such as DATADOG_CREATE_MONITOR), so sendEvent can proceed to call doPost
with app-key=false.
- Around line 760-777: In muteMonitor, validate that req.Filter contains a
well-formed monitor identifier and escape it before building apiPath;
specifically, ensure the monitorID from req.Filter is checked against the
expected pattern (e.g., digits or allowed chars) and then apply a path-escaping
function (use url.PathEscape or equivalent) to the validated monitorID before
calling fmt.Sprintf("/api/v1/monitor/%s/mute", monitorID) so the constructed
apiPath cannot be manipulated by malformed input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd4cee6f-8091-4fac-88f7-1d9674c88e60
⛔ Files ignored due to path filters (1)
api/gen/cloud/v1/message.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (2)
api/cloud/v1/message.protopkg/datadog_executor/datadog_executor.go
Datadog queries previously only worked over the control stream. In multi-replica server deployments, the control stream may be on a different replica, causing workflow Datadog queries to fail. Proto: - Add DatadogQueryRequest to StreamDataResponse (field 11) - Add DatadogQueryResponse to StreamDataRequest (field 26) Operator: - Add handleDatadogQueryRequestOnDataStream that processes Datadog queries received on the data stream and sends responses back - Wire into data stream receive loop Made-with: Cursor
New argocd_executor package: discovers argocd-server service in the cluster, authenticates via admin secret, and executes sync/status/list operations through the in-cluster ArgoCD API. - Add ARGOCD_SYNC_APP=10, ARGOCD_GET_APP_STATUS=11, ARGOCD_LIST_APPS=12 to DatadogQueryType enum (reuses existing request/response messages) - Add has_argocd field to InventoryCommit - Gate executor creation on ARGOCD_NAMESPACE env var (set via Helm) - Report has_argocd in InventoryCommit for server-side awareness - Dispatch ArgoCD query types through existing Datadog gRPC channel - Add DATADOG_LIST_MONITORS=9 query type for DD monitor dropdown Made-with: Cursor
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
pkg/argocd_executor/argocd_executor.go (4)
443-450:StatusCodemay be 0 when HTTP request fails.When
err != nil(e.g., connection refused, timeout),statusCodepassed tomakeResponseis 0. This results inresp.StatusCode = 0, which may be confusing for consumers expecting a valid HTTP status.Consider defaulting to a meaningful status code like 502 (Bad Gateway) or 503 (Service Unavailable) for transport errors:
♻️ Proposed fix
func (e *ArgoCDExecutor) makeResponse(requestID string, body []byte, statusCode int, err error) *v1.DatadogQueryResponse { resp := &v1.DatadogQueryResponse{RequestId: requestID} if err != nil { resp.Success = false resp.ErrorMessage = err.Error() - resp.StatusCode = int32(statusCode) + if statusCode == 0 { + resp.StatusCode = http.StatusBadGateway // 502 for transport errors + } else { + resp.StatusCode = int32(statusCode) + } return resp }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 443 - 450, The makeResponse function currently sets resp.StatusCode directly from the statusCode argument, which can be 0 on transport errors; update ArgoCDExecutor.makeResponse to detect when err != nil and statusCode == 0 and set resp.StatusCode to a sensible default (e.g., 502 or 503) instead of 0; ensure you still set resp.Success=false and resp.ErrorMessage=err.Error() and return the response as before, referencing the makeResponse method, resp.StatusCode field, and the error handling branch to locate the change.
178-186: Redundant nil check after successful Get.If
e.ClientSet.CoreV1().Services(ns).Get()returnserr == nil,svcis guaranteed to be non-nil. The check on line 182 is unnecessary.♻️ Proposed simplification
for _, ns := range argoCDCommonNamespaces { svc, err := e.ClientSet.CoreV1().Services(ns).Get(ctx, "argocd-server", metav1.GetOptions{}) if err != nil { continue } - if svc != nil { - e.Logger.Info("[ArgoCD] Found argocd-server service", - zap.String("namespace", ns)) - return ns, nil - } + e.Logger.Info("[ArgoCD] Found argocd-server service", + zap.String("namespace", ns)) + return ns, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 178 - 186, The nil check on svc after a successful call to e.ClientSet.CoreV1().Services(ns).Get(ctx, "argocd-server", metav1.GetOptions{}) is redundant; update the logic so that after the Get call you only continue when err != nil, and otherwise immediately log the found service and return the namespace (remove the if svc != nil block and directly call e.Logger.Info("[ArgoCD] Found argocd-server service", zap.String("namespace", ns)) followed by return ns, nil).
239-251: Reliance onargocd-initial-admin-secretmay break in hardened setups.The
argocd-initial-admin-secretis often deleted after initial ArgoCD setup in production environments following security best practices. If deleted, this executor will fail to authenticate.Consider:
- Documenting this requirement for operators
- Adding fallback to a dedicated service account or configurable secret name
- Supporting ArgoCD's OIDC/SSO authentication in a future iteration
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 239 - 251, Update getAdminPassword to avoid hardcoding "argocd-initial-admin-secret": first attempt to read a configurable secret name (e.g., use a new field like e.adminSecretName or config.AdminSecretName) and return if found; if not present, fall back to fetching a service account token (e.g., read the ServiceAccount token from the ArgoCD server/service account via e.ClientSet.CoreV1().Secrets or ServiceAccounts) and use that for auth; ensure errors enumerate both attempted sources in the returned error and document the requirement in README/operator docs. Reference getAdminPassword, e.ClientSet, and add the new config/field to ArgoCDExecutor so callers can override the secret name.
270-274: Unchecked error fromresp.Body.Close().Static analysis flags that the error from
resp.Body.Close()is not checked. While typically benign, it can be addressed by using a deferred function that logs errors.♻️ Proposed fix
- defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + e.Logger.Debug("[ArgoCD] Failed to close response body", zap.Error(err)) + } + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 270 - 274, Replace the bare defer resp.Body.Close() with a deferred anonymous function that calls resp.Body.Close(), checks its returned error, and logs it; locate the call after e.httpClient.Do(req) (the resp variable) and implement the defer to capture and handle the Close() error (use the existing logger on the executor instance, e.g. e.logger.Errorf or fall back to fmt.Printf/fmt.Errorf if no logger is present) so Close errors are not ignored.api/cloud/v1/message.proto (1)
1339-1341: ArgoCD query types are semantically misplaced inDatadogQueryType.Mixing ArgoCD operations (
ARGOCD_SYNC_APP,ARGOCD_GET_APP_STATUS,ARGOCD_LIST_APPS) intoDatadogQueryTypecreates semantic confusion. This enum now serves as a general "external tool query type" rather than Datadog-specific.Consider renaming the enum to
ExternalQueryTypeorToolQueryTypeto reflect its broader purpose, or introduce a separateArgoCDQueryTypeenum. This would improve clarity and maintainability as more integrations are added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/cloud/v1/message.proto` around lines 1339 - 1341, The enum DatadogQueryType incorrectly includes ARGOCD_SYNC_APP, ARGOCD_GET_APP_STATUS, and ARGOCD_LIST_APPS, which broadens its responsibility; rename DatadogQueryType to a more generic name like ExternalQueryType (or create a new ArgoCDQueryType and move the ARGOCD_* members there) and update all usages/references to the enum and its members (e.g., DatadogQueryType -> ExternalQueryType or switch ARGOCD_SYNC_APP, ARGOCD_GET_APP_STATUS, ARGOCD_LIST_APPS into a new ArgoCDQueryType) so the API semantics are clear and Datadog-specific naming is preserved for Datadog-only queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/argocd_executor/argocd_executor.go`:
- Around line 326-328: The code constructs the API path by interpolating appName
from req.Filter directly (see variable appName and the path passed to e.doGet
and later at the other occurrence around lines 351-352), which allows path
traversal; fix by validating/escaping the appName before building the path—use
url.PathEscape(appName) (or another strict validation) when creating the path
string for e.doGet and any other place that formats "/api/v1/applications/%s" so
the resulting path is safe and special characters (e.g., "/", "..") are encoded.
- Around line 51-56: The TLS config for the httpClient currently sets
InsecureSkipVerify: true without enforcing a minimum TLS version; update the
tls.Config used in the httpClient's Transport (the httpClient and its Transport
struct) to set MinVersion to tls.VersionTLS12 (or tls.VersionTLS13) and make
InsecureSkipVerify configurable (via a flag/env var or a parameter) and
documented; adjust the code that constructs the httpClient to read the new
config option and set tls.Config{InsecureSkipVerify: cfg.InsecureSkipVerify,
MinVersion: tls.VersionTLS12} (or equivalent) so modern TLS is enforced while
preserving the optional bypass for self-signed certs.
---
Nitpick comments:
In `@api/cloud/v1/message.proto`:
- Around line 1339-1341: The enum DatadogQueryType incorrectly includes
ARGOCD_SYNC_APP, ARGOCD_GET_APP_STATUS, and ARGOCD_LIST_APPS, which broadens its
responsibility; rename DatadogQueryType to a more generic name like
ExternalQueryType (or create a new ArgoCDQueryType and move the ARGOCD_* members
there) and update all usages/references to the enum and its members (e.g.,
DatadogQueryType -> ExternalQueryType or switch ARGOCD_SYNC_APP,
ARGOCD_GET_APP_STATUS, ARGOCD_LIST_APPS into a new ArgoCDQueryType) so the API
semantics are clear and Datadog-specific naming is preserved for Datadog-only
queries.
In `@pkg/argocd_executor/argocd_executor.go`:
- Around line 443-450: The makeResponse function currently sets resp.StatusCode
directly from the statusCode argument, which can be 0 on transport errors;
update ArgoCDExecutor.makeResponse to detect when err != nil and statusCode == 0
and set resp.StatusCode to a sensible default (e.g., 502 or 503) instead of 0;
ensure you still set resp.Success=false and resp.ErrorMessage=err.Error() and
return the response as before, referencing the makeResponse method,
resp.StatusCode field, and the error handling branch to locate the change.
- Around line 178-186: The nil check on svc after a successful call to
e.ClientSet.CoreV1().Services(ns).Get(ctx, "argocd-server", metav1.GetOptions{})
is redundant; update the logic so that after the Get call you only continue when
err != nil, and otherwise immediately log the found service and return the
namespace (remove the if svc != nil block and directly call
e.Logger.Info("[ArgoCD] Found argocd-server service", zap.String("namespace",
ns)) followed by return ns, nil).
- Around line 239-251: Update getAdminPassword to avoid hardcoding
"argocd-initial-admin-secret": first attempt to read a configurable secret name
(e.g., use a new field like e.adminSecretName or config.AdminSecretName) and
return if found; if not present, fall back to fetching a service account token
(e.g., read the ServiceAccount token from the ArgoCD server/service account via
e.ClientSet.CoreV1().Secrets or ServiceAccounts) and use that for auth; ensure
errors enumerate both attempted sources in the returned error and document the
requirement in README/operator docs. Reference getAdminPassword, e.ClientSet,
and add the new config/field to ArgoCDExecutor so callers can override the
secret name.
- Around line 270-274: Replace the bare defer resp.Body.Close() with a deferred
anonymous function that calls resp.Body.Close(), checks its returned error, and
logs it; locate the call after e.httpClient.Do(req) (the resp variable) and
implement the defer to capture and handle the Close() error (use the existing
logger on the executor instance, e.g. e.logger.Errorf or fall back to
fmt.Printf/fmt.Errorf if no logger is present) so Close errors are not ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4f5e5f9-9ab5-4ea7-902a-d4ef95f3355c
⛔ Files ignored due to path filters (1)
api/gen/cloud/v1/message.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (4)
api/cloud/v1/message.protopkg/argocd_executor/argocd_executor.gopkg/client/stream_client.gopkg/datadog_executor/datadog_executor.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/datadog_executor/datadog_executor.go
- pkg/client/stream_client.go
- ArgoCD executor: use url.PathEscape for app name in API paths to prevent path traversal - Datadog executor: use url.PathEscape for monitor ID in mute path - Datadog executor: allow sendEvent to bypass app-key gate since it only needs DD-API-KEY (doPost with needsAppKey=false) Made-with: Cursor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/argocd_executor/argocd_executor.go (1)
52-57:⚠️ Potential issue | 🟠 MajorAdd
MinVersionto TLS configuration.The TLS configuration sets
InsecureSkipVerify: truewithout enforcing a minimum TLS version. AddMinVersion: tls.VersionTLS12(ortls.VersionTLS13) to prevent downgrade attacks.🛡️ Proposed fix
httpClient: &http.Client{ Timeout: 30 * time.Second, Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, // ArgoCD may use self-signed certs in-cluster + MinVersion: tls.VersionTLS12, + }, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 52 - 57, The TLS config used in the HTTP client (the httpClient field where Transport is set and TLSClientConfig is created) currently sets InsecureSkipVerify: true but lacks a minimum TLS version; update the tls.Config in that Transport's TLSClientConfig to include MinVersion: tls.VersionTLS12 (or tls.VersionTLS13) to enforce a minimum TLS version and prevent downgrade attacks.
🧹 Nitpick comments (4)
pkg/argocd_executor/argocd_executor.go (4)
386-386: Unchecked error return fromresp.Body.Close.Same as in the
loginmethod—consider wrapping with error logging for consistency.♻️ Proposed fix for doGet
- defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + e.Logger.Debug("[ArgoCD] Failed to close response body", zap.Error(closeErr)) + } + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` at line 386, The resp.Body.Close() call in doGet is ignoring its returned error (same issue as in login); update doGet to capture and handle the Close error for consistency with login by replacing the bare defer resp.Body.Close() with a deferred closure that checks the error (e.g., if err := resp.Body.Close(); err != nil { <use the executor's logger or error reporting used in login> to log the error }) so any Close failures are logged; reference resp.Body.Close, doGet, and login when locating the change.
428-428: Unchecked error return fromresp.Body.Close.Apply the same fix pattern here for
doPost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` at line 428, The defer resp.Body.Close() call in doPost ignores the returned error; replace it with a deferred closure that captures and handles the Close error (e.g. defer func() { if err := resp.Body.Close(); err != nil { /* log or wrap the error via the existing logger or return it */ } }()) so the Close error is not silently dropped—locate the resp variable in the doPost function and apply the same error-checking pattern used elsewhere in this file.
177-204: Minor: redundant nil check for service.On line 183,
if svc != nilis redundant—whenGetreturnsnilerror,svcis guaranteed to be non-nil.♻️ Optional cleanup
for _, ns := range argoCDCommonNamespaces { svc, err := e.ClientSet.CoreV1().Services(ns).Get(ctx, "argocd-server", metav1.GetOptions{}) if err != nil { continue } - if svc != nil { - e.Logger.Info("[ArgoCD] Found argocd-server service", - zap.String("namespace", ns)) - return ns, nil - } + e.Logger.Info("[ArgoCD] Found argocd-server service", + zap.String("namespace", ns)) + return ns, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 177 - 204, The nil check for svc in findArgoCDNamespace is redundant; when e.ClientSet.CoreV1().Services(ns).Get(...) returns no error, svc is non-nil. Remove the if svc != nil branch and move the log/return directly after the successful Get (inside the loop over argoCDCommonNamespaces), keeping the existing log message using e.Logger.Info and returning ns, nil; leave the cluster-wide List logic (svcList iteration) unchanged.
271-275: Unchecked error return fromresp.Body.Close.The error returned by
resp.Body.Close()is ignored. While rarely critical for reads, it's good practice to log or handle the error, especially for consistency.♻️ Proposed fix
- defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + e.Logger.Debug("[ArgoCD] Failed to close response body", zap.Error(err)) + } + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/argocd_executor/argocd_executor.go` around lines 271 - 275, The call to resp.Body.Close() after e.httpClient.Do(req) currently ignores its error; update the code to capture and handle that return value (e.g., closeErr := resp.Body.Close(); if closeErr != nil { /* log or return wrapped error */ }), using the executor's logging facility if available (e.g., e.logger.Errorf) or returning a wrapped error from the surrounding function. Locate the HTTP call where resp is obtained from e.httpClient.Do(req) and replace the blind defer resp.Body.Close() with a defer that checks and handles the error from resp.Body.Close().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/argocd_executor/argocd_executor.go`:
- Around line 52-57: The TLS config used in the HTTP client (the httpClient
field where Transport is set and TLSClientConfig is created) currently sets
InsecureSkipVerify: true but lacks a minimum TLS version; update the tls.Config
in that Transport's TLSClientConfig to include MinVersion: tls.VersionTLS12 (or
tls.VersionTLS13) to enforce a minimum TLS version and prevent downgrade
attacks.
---
Nitpick comments:
In `@pkg/argocd_executor/argocd_executor.go`:
- Line 386: The resp.Body.Close() call in doGet is ignoring its returned error
(same issue as in login); update doGet to capture and handle the Close error for
consistency with login by replacing the bare defer resp.Body.Close() with a
deferred closure that checks the error (e.g., if err := resp.Body.Close(); err
!= nil { <use the executor's logger or error reporting used in login> to log the
error }) so any Close failures are logged; reference resp.Body.Close, doGet, and
login when locating the change.
- Line 428: The defer resp.Body.Close() call in doPost ignores the returned
error; replace it with a deferred closure that captures and handles the Close
error (e.g. defer func() { if err := resp.Body.Close(); err != nil { /* log or
wrap the error via the existing logger or return it */ } }()) so the Close error
is not silently dropped—locate the resp variable in the doPost function and
apply the same error-checking pattern used elsewhere in this file.
- Around line 177-204: The nil check for svc in findArgoCDNamespace is
redundant; when e.ClientSet.CoreV1().Services(ns).Get(...) returns no error, svc
is non-nil. Remove the if svc != nil branch and move the log/return directly
after the successful Get (inside the loop over argoCDCommonNamespaces), keeping
the existing log message using e.Logger.Info and returning ns, nil; leave the
cluster-wide List logic (svcList iteration) unchanged.
- Around line 271-275: The call to resp.Body.Close() after e.httpClient.Do(req)
currently ignores its error; update the code to capture and handle that return
value (e.g., closeErr := resp.Body.Close(); if closeErr != nil { /* log or
return wrapped error */ }), using the executor's logging facility if available
(e.g., e.logger.Errorf) or returning a wrapped error from the surrounding
function. Locate the HTTP call where resp is obtained from e.httpClient.Do(req)
and replace the blind defer resp.Body.Close() with a defer that checks and
handles the error from resp.Body.Close().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44b14f64-8de1-4da4-907d-7302d38d8e67
📒 Files selected for processing (2)
pkg/argocd_executor/argocd_executor.gopkg/datadog_executor/datadog_executor.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/datadog_executor/datadog_executor.go
Add argocd.enabled and argocd.namespace to values.yaml and the ARGOCD_NAMESPACE env var to the deployment template, matching the Datadog integration pattern. Made-with: Cursor
… query types Made-with: Cursor
… Workflows Major documentation rewrite aligned with the new "AI Agents for Cloud Operations" tagline and devtools aesthetic. Theme: Updated mint.json colors from blue to neutral grays matching the platform UI redesign (#0A0A0A dark bg, #171717 primary, #FFFFFF light bg). New structure: - Get Started: Introduction + 3 product-specific quickstarts (Workflows, Incident Response, Cloud AI Copilot) - Workflows: Create Workflows, Custom Integrations, Setup, Observability - Incident Response: Kubernetes + Cloud - Cloud AI Copilot: Overview - Integrations: 17 integration pages (Kubernetes, AWS, OCI, Slack, GitHub, GitLab, PagerDuty, Jira, Confluence, Glean, Linear, PostHog, Vercel, Datadog, OpenTelemetry, ArgoCD, Knowledge Sources) - Enterprise: Security & Compliance + On-Premise Removed risk assessment references. Workflows positioned as primary product throughout. Old pages preserved with redirects to new locations. Made-with: Cursor
…cons Colors: use zinc-900 (#18181B) as primary and zinc-50 (#FAFAFA) as dark accent so the Dashboard CTA button is visible in both light and dark modes. Background adjusted to zinc-950 (#09090B). Icons: replace invalid "message-bot" with "comments", "aws" with "cloud" for Font Awesome compatibility. Made-with: Cursor
HTML comments (<!-- -->) are not supported in MDX and cause build
failures. Converted all screenshot placeholder comments to JSX
syntax ({/* */}) across all 10 affected MDX files.
Made-with: Cursor
…rkflow details Made-with: Cursor
…ming, webhook triggers Made-with: Cursor
…listic queries Made-with: Cursor
…NL prompts Made-with: Cursor
…e false SDK claim Made-with: Cursor
…fix knowledge sources Made-with: Cursor
…incident types Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Complete rewrite of docs/workflows/sdk.mdx: - Remove fake TypeScript SDK references (doesn't exist) - Document API key authentication as primary auth method - Full builder pattern guide (Trigger, Action, Condition, Approval) - All trigger factory methods with filter chaining examples - All action factory methods grouped by integration - Branching and approval examples - AsyncKestrelClient with full async/await example - executions.wait() for polling execution status - API key scopes reference table - Error handling with exception hierarchy Made-with: Cursor
Made-with: Cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Adds ARGOCD_ROLLBACK_APP (type=13) to the DatadogQueryType enum and
implements the rollbackApp handler in ArgoCDExecutor. This enables
workflow automation to roll back ArgoCD applications to previous
versions via POST /api/v1/applications/{name}/rollback.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Adds DatadogMonitorPoller that polls GET /api/v1/monitor every 60s using existing DD API credentials, detects state transitions, and sends DatadogMonitorAlert signals over the gRPC stream to the server. - Proto: DatadogMonitorAlert message in StreamEventsRequest + StreamDataRequest - MonitorPoller: polls monitors, tracks state, emits on transitions - StreamClient: creates channel, starts poller after DD probe, consumes alerts in sendIncidentData with sendEventOrFallback pattern Co-authored-by: Cursor <cursoragent@cursor.com>
- Datadog: added trigger blocks section (monitor alerts via operator polling), updated description - ArgoCD: added Rollback and Find App for Workload blocks - GitHub: added Investigate GitHub Action Failure block - SDK docs: added argocd_find_app, github_investigate_action_failure, and all Datadog trigger factories with filter examples Co-authored-by: Cursor <cursoragent@cursor.com>
Installs Helm v3.16.3 (multi-arch) alongside kubectl, cilium, and trivy. Enables workflow blocks to execute helm install/upgrade/rollback/uninstall commands via the shell executor pipeline. Co-authored-by: Cursor <cursoragent@cursor.com>
- New: docs/integrations/helm.mdx with all 5 blocks documented, example workflows, chart source reference, and troubleshooting - Updated: SDK docs with helm_upgrade, helm_install, helm_rollback, helm_uninstall, helm_status factory method examples Co-authored-by: Cursor <cursoragent@cursor.com>
Helm needs to create/update/delete Kubernetes resources when managing releases (secrets for release metadata, deployments, services, etc.). Added write permissions for common resource types that Helm charts typically create. Co-authored-by: Cursor <cursoragent@cursor.com>
…ing one Co-authored-by: Cursor <cursoragent@cursor.com>
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
2 participants
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.
Extend the Datadog integration to support write operations (create monitor, send event, mute monitor) in addition to the existing read-only queries, for use by the workflow builder's Datadog actions.
Proto (message.proto):
Operator (datadog_executor.go):
Summary by CodeRabbit
New Features
Behavior