Skip to content

[pull] master from DataDog:master#551

Merged
pull[bot] merged 7 commits into
ConnectionMaster:masterfrom
DataDog:master
May 20, 2026
Merged

[pull] master from DataDog:master#551
pull[bot] merged 7 commits into
ConnectionMaster:masterfrom
DataDog:master

Conversation

@pull

@pull pull Bot commented May 20, 2026

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

AAraKKe and others added 7 commits May 20, 2026 14:37
* Require an explicit QA decision label on every pull request

Adds a 'ddev validate qa-label' check that fails the validate-all CI job
unless the PR has exactly one of qa/required or qa/skip-qa. Removes the
old advisory validate-skip-qa workflow and its render composite action,
which only posted a comment and did not block merges.

The new validation is a no-op outside pull_request events and on
fork PRs (where the token cannot read labels).

* Read PR labels through the direct pull-request endpoint and fail closed on bad input

- Add GitHubManager.get_pull_request_labels(pr_number), backed by the direct
  /pulls/{number} endpoint. The previous helper used the fuzzy search API,
  which could return a different PR when one number is a substring of another.
- Abort qa-label when GITHUB_EVENT_PATH is set but unparseable; silently
  skipping would defeat the gate.
- Use a frozenset for the accepted-label set and split the chained get-or-{}
  walk in _is_fork_pr into named intermediates.
- Add tests covering the malformed-event, missing-PR-number, and
  unfetchable-labels branches.
- Fix the AGENTS.md "e.g." spelling to match the rest of the file.
- Add the PR-23748 changelog entry the CI changelog check was waiting on.

* Fail closed when the pull_request event payload is malformed

- Raise on a non-object JSON payload in _load_event_payload so a syntactically
  valid but structurally wrong file (e.g. a JSON array or string) hits the
  same abort path as a parse failure instead of crashing inside _is_fork_pr
  with an AttributeError.
- Abort in _is_fork_pr when head/base repo info is missing, matching the
  fail-closed shape used everywhere else in this module.
- Extract a shared extract_pr_number(event) helper in validate/all/github.py
  so qa_label and parse_pr_number_from_event no longer duplicate the
  pull_request.number walk.
- Drop the unused -> None annotation on the click entry point and parametrize
  the unreadable-event test to cover the missing-file and non-object cases.

* Raise ValueError for malformed event payloads instead of TypeError

* Parse the GitHub Actions event payload through a typed model

* Rename GitHubEvent to PullRequestEvent to match what it models

* Re-run repo validations when PR labels change so the qa-label gate updates
* Add GitHub Actions annotation primitive to ddev's Application

Adds Application.annotate_error/annotate_warning/annotate_display_queue
that emit GitHub Actions workflow annotations (::error/::warning) when
running on CI and no-op locally. Mirrors the legacy
datadog_checks_dev/tooling/commands/console.py helpers so future
migrations (and a retrofit of #23651/#23652/#23653) keep CI annotation
UX.

The display-queue helper takes a list of (level, message) tuples instead
of the legacy (func, message) shape, which is idiomatic for ddev's
Application-style API.

* Add changelog

* Address review feedback: StrEnum + defaultdict

- Replace ANNOTATION_LEVEL_ERROR/WARNING string constants and the
  _GH_ANNOTATION_LEVELS frozenset with a single AnnotationLevel(StrEnum).
  Method signatures now type level as AnnotationLevel; the old runtime
  "unknown level falls back to error" guard is gone because the enum
  enforces membership at construction time.
- Swap annotate_display_queue's pre-populated dict + membership filter
  for collections.defaultdict(list). Emission iterates AnnotationLevel
  so output order is deterministic (errors before warnings) regardless
  of input order — covered by a new test.
- Add a one-line comment near the print() call explaining why it isn't
  os.system("echo ...") (the legacy shape was shell-injection prone).
- Tests use AnnotationLevel members directly; new tests for StrEnum
  string-identity and the deterministic emission order. 12 tests total.

* Escape workflow-command data and property values

The previous implementation interpolated message and property values raw
into the workflow command, so a literal newline in the message would
terminate the annotation early, a `%0A` would be decoded back into a
newline, and `,` / `:` in a file path would corrupt the property list.

Add `_escape_workflow_data` and `_escape_workflow_property` private
helpers that mirror `@actions/core`'s `escapeData` / `escapeProperty`:

- Data:     % -> %25, \r -> %0D, \n -> %0A
- Property: same, plus : -> %3A, , -> %2C

Verified against actions/toolkit packages/core/src/command.ts (same
operations, same order). `annotate_display_queue` now joins messages
with a real `\n` so the escaper produces a single canonical `%0A`
sequence instead of double-encoding a literal `%0A`.

Tests cover newline / percent / property-separator round-trips plus
table-driven coverage of each escaper helper. Existing ASCII-only tests
keep passing because the escapers are identity on those inputs.

* Move annotation helpers to ddev.utils.ci

- Relocate AnnotationLevel, escape_workflow_data, escape_workflow_property from cli.application to utils.ci so future callers depend on the utilities layer rather than the application layer
- Widen annotate_display_queue's display_queue parameter to Iterable
- Drop the dead escape on str(line) and add a test pinning that unknown levels are skipped

* Condense annotation safety comment to one line
* Update integration references to claude compliance.

add installation source

changelog

* claude compliance API

* Apply suggestion from @brett0000FF

Co-authored-by: Brett Blue <84536271+brett0000FF@users.noreply.github.com>

* Apply suggestion from @brett0000FF

Co-authored-by: Brett Blue <84536271+brett0000FF@users.noreply.github.com>

* Apply suggestion from @brett0000FF

Co-authored-by: Brett Blue <84536271+brett0000FF@users.noreply.github.com>

---------

Co-authored-by: Brett Blue <84536271+brett0000FF@users.noreply.github.com>
…tname for loopback addresses (#23756)

* ClickHouse: honor reported_hostname config and fall back to agent hostname for loopback addresses

Two related changes:

1. ClickHouse: fix `reported_hostname` being ignored
   The ClickHouse check was not reading the `reported_hostname` config option.
   When the agent is co-located with ClickHouse (e.g. same pod), `127.0.0.1`
   was sent to the host pipeline — not a unique hostname.

2. Lift `reported_hostname` resolution into `DatabaseCheck` base class
   All four DBM checks (Postgres, MySQL, SQL Server, ClickHouse) independently
   implemented the same three-step resolution logic:
   - If `exclude_hostname` → return None
   - If `reported_hostname` config is set → use it directly
   - Otherwise → call `resolve_db_host(host)` to substitute agent hostname
     for loopback addresses

   This PR moves that logic once into `DatabaseCheck`: `reported_hostname` is
   the public property that gates on `exclude_hostname`, and `resolved_hostname`
   is a new concrete base property that handles the config/loopback resolution
   and caches the result. Each check now implements a one-line abstract
   `_config_host` returning its config's host/server field.

   Note: `_config_host` is a breaking change for any external `DatabaseCheck`
   subclasses — they must now implement it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ClickHouse: honor reported_hostname config and fall back to agent hostname for loopback addresses

The ClickHouse check was not reading the `reported_hostname` config option.
The k8s-resources template for `dbm-agent-integration` already renders it
into the agent config when set in the values YAML, but the check silently
ignored it and always used `self._config.server` directly. When the agent is
co-located with ClickHouse (e.g. same pod), this meant `127.0.0.1` was sent
to the host pipeline, which is not a unique hostname and causes host resolver
warnings.

Fix `reported_hostname` to apply the same three-step resolution used by the
other DBM checks (Postgres, MySQL, SQL Server):
1. If `reported_hostname` config is set → use it directly
2. Otherwise → call `resolve_db_host(server)` which substitutes the agent
   hostname for loopback addresses (localhost, 127.0.0.1)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Bump urllib3 from 2.6.3 to 2.7.0

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Add changelog entry

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Sync agent_requirements.in for urllib3 bump

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Update dependency resolution

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: dd-agent-integrations-bot[bot] <dd-agent-integrations-bot[bot]@users.noreply.github.com>
@pull pull Bot locked and limited conversation to collaborators May 20, 2026
@pull pull Bot added the ⤵️ pull label May 20, 2026
@pull pull Bot merged commit 152b9cb into ConnectionMaster:master May 20, 2026
1 check passed
@pull pull Bot temporarily deployed to release May 20, 2026 20:28 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants