Skip to content

fix(CONTP-2001): stop asserting the Operator image tag in baseline tests (operator chart) - #2897

Open
nlchung wants to merge 17 commits into
mainfrom
nicole.chung/contp-2001
Open

fix(CONTP-2001): stop asserting the Operator image tag in baseline tests (operator chart)#2897
nlchung wants to merge 17 commits into
mainfrom
nicole.chung/contp-2001

Conversation

@nlchung

@nlchung nlchung commented Sep 4, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

The datadog-operator baseline tests asserted on the image tag, so every Operator release broke the baseline for no reason — the release script just auto-patched the tag away, catching nothing.

  • Strip the image tag (and app.kubernetes.io/version label) before comparing baselines, via a new utils.ImageRepository helper and Normalizer callback support in VerifyBaseline.
  • Replace the hardcoded tag assertion in operator_deployment_test.go with a dynamic check against the Deployment's own version label — still catches a release that updates one but not the other.
  • Split the baseline test into Test_baseline_deployment and Test_baseline_crd, since only the CRD baseline still needs release-time regeneration (see below).
  • Remove the release-worker steps that rewrote the tag and regenerated the Deployment baseline — no longer needed now that both are normalized. The CRD baseline is still auto-regenerated (via a new make update-test-baselines-operator-crd target scoped to Test_baseline_crd), since a real datadog-crds schema change is a real content change, not noise — see notes below.
  • Minor: fixed VerifyBaseline's failure output to not show a fake helm.sh/chart diff line.

Verified locally: a throwaway image.tag bump no longer requires a baseline change; a real Deployment template change still fails the test.

Which issue this PR fixes

CONTP-2001

Special notes for your reviewer:

No chart behavior changes — test assertions and the release script only. CONTP-2001 is scoped to the Deployment baseline/tag; it says nothing about the CRD baseline, so that behavior is unchanged (still auto-regenerated), just scoped so it can't also mask a Deployment regression.

Kept the CRD auto-regen deliberately: any datadog-crds schema change goes through its own reviewed PR in the earlier crds release phase (see release runbook, and the required phase input in operator-release-chart-scripts.yaml) before this later operator-phase step ever runs — so the schema change itself is already reviewed by the time this baseline update happens.

Checklist

  • All commits are signed and show as "Verified" on GitHub (see: [signing commits][1])
  • For datadog or datadog-operator chart or value changes, update the test baselines (run: make update-test-baselines)

@nlchung nlchung self-assigned this Sep 8, 2026
@nlchung nlchung added the datadog-operator/no-version-bump Skip version bump for datadog-operator chart label Sep 8, 2026
@nlchung nlchung changed the title CONTP-2001 Stop asserting the Operator image tag in helm-charts baseline tests fix(CONTP-2001): stop asserting the Operator image tag in baseline tests (operator chart) Sep 8, 2026
@nlchung
nlchung force-pushed the nicole.chung/contp-2001 branch from 0b433e6 to d83f6f3 Compare September 9, 2026 14:30
@nlchung
nlchung marked this pull request as ready for review September 9, 2026 14:34
@nlchung
nlchung requested review from a team as code owners September 9, 2026 14:34
@nlchung
nlchung requested review from fanny-jiang and removed request for a team September 9, 2026 14:34
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T14:39:38.122864Z d83f6f3 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d83f6f3372

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/release-operator.sh Outdated
Comment thread test/datadog-operator/operator_deployment_test.go Outdated
@datadog-datadog-prod-us1

This comment has been minimized.

The CRD baseline comparison has no normalizer, so it still needs
regenerating whenever datadog-crds brings in a real schema change -
restore that step in phase_operator. Also restore validation that the
default Operator image tag tracks Chart.AppVersion (via the
app.kubernetes.io/version label) instead of dropping tag validation
entirely, so a release bump that updates one but not the other is
still caught.
…sions

make update-test-baselines-operator regenerates both baselines
unconditionally, which would silently re-absorb a real Deployment
template regression into the baseline if it landed in the same
release commit - exactly what the Deployment normalizer was meant to
prevent. Add update-test-baselines-operator-crd, scoped via -run to
only the CRD subtest, and use that in phase_operator instead.
nlchung and others added 8 commits September 9, 2026 14:09
…ine_crd

The two baseline checks have different regen semantics: Deployment is
normalized against release-version noise and never needs regenerating on
release, while CRD is a literal diff that must be regenerated on every real
schema change. They were table rows of one shared test, which forced
update-test-baselines-operator-crd to scope itself via `go test -run` against
a table row's human-readable display string - a coupling that silently
matches nothing if that string is ever renamed. Splitting them into named
top-level test functions lets the Makefile target scope against a stable,
compile-visible identifier instead.
CONTP-2001's acceptance criteria say the release worker should not edit
baseline files, since a worker that silently rewrites baselines just moves
the churn rather than removing it. The CRD baseline is a literal diff (no
version normalization), so unlike the Deployment case, a real datadog-crds
schema change legitimately needs a human to review the diff before it ships.
Letting unit-test-operator fail on such a change, rather than having this
script auto-regenerate the baseline, is that checkpoint, not a bug -
`make update-test-baselines-operator-crd` stays available for the release
engineer to run by hand once they've looked at what changed.
The comment above update-test-baselines-operator-crd still described a
CRD-baseline auto-regen "automated release commit" that no longer exists
now that release-operator.sh deliberately does not call this target
(CONTP-2001). Correct it to describe this as manual tooling.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CONTP-2001's acceptance criteria are scoped to the Deployment baseline
and image tag only; they say nothing about the CRD baseline. Codex's
review correctly flagged that removing CRD baseline regeneration
entirely would break unit-test-operator on any release that advances
datadog-crds with a real schema change. Restore that regeneration,
scoped to Test_baseline_crd only so it can't also silently absorb a
real Deployment template regression into the same commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The CRD baseline comparison is a typed struct diff (via cmp.Equal),
not a literal text diff - cosmetic differences don't fail it. Also
note that the schema change it reacts to already went through its
own reviewed PR in the earlier crds release phase.
The prior comments were long design explanations that belong in the
PR description, not the code.
@nlchung nlchung added tools/ci and removed tools/ci labels Sep 10, 2026
nlchung and others added 3 commits September 10, 2026 10:41
Drop the write-gate/split-test complexity in favor of the original
single Test_baseline_manifests, keeping only the normalizer needed to
stop comparing the Operator image tag and version label.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datadog-operator/no-version-bump Skip version bump for datadog-operator chart tools/ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant