fix(CONTP-2001): stop asserting the Operator image tag in baseline tests (operator chart) - #2897
Open
nlchung wants to merge 17 commits into
Open
fix(CONTP-2001): stop asserting the Operator image tag in baseline tests (operator chart)#2897nlchung wants to merge 17 commits into
nlchung wants to merge 17 commits into
Conversation
2 tasks
nlchung
force-pushed
the
nicole.chung/contp-2001
branch
from
September 9, 2026 14:30
0b433e6 to
d83f6f3
Compare
nlchung
marked this pull request as ready for review
September 9, 2026 14:34
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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".
This comment has been minimized.
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.
…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.
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>
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
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.
What this PR does / why we need it:
The
datadog-operatorbaseline 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.app.kubernetes.io/versionlabel) before comparing baselines, via a newutils.ImageRepositoryhelper andNormalizercallback support inVerifyBaseline.operator_deployment_test.gowith a dynamic check against the Deployment's own version label — still catches a release that updates one but not the other.Test_baseline_deploymentandTest_baseline_crd, since only the CRD baseline still needs release-time regeneration (see below).make update-test-baselines-operator-crdtarget scoped toTest_baseline_crd), since a realdatadog-crdsschema change is a real content change, not noise — see notes below.VerifyBaseline's failure output to not show a fakehelm.sh/chartdiff line.Verified locally: a throwaway
image.tagbump 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-crdsschema change goes through its own reviewed PR in the earliercrdsrelease phase (see release runbook, and the requiredphaseinput inoperator-release-chart-scripts.yaml) before this lateroperator-phase step ever runs — so the schema change itself is already reviewed by the time this baseline update happens.Checklist
datadogordatadog-operatorchart or value changes, update the test baselines (run:make update-test-baselines)