MON-4578: Add new fields to ThanosQuerierConfig by danielmellado · Pull Request #2835 · openshift/api

danielmellado · 2026-05-07T12:29:02Z

Port ThanosQuerierConfig fields from the CMO ConfigMap API to the
ClusterMonitoring CRD. LogLevel reuses the existing LogLevel enum
(Error, Warn, Info, Debug). EnableRequestLogging and EnableCORS use
a new ThanosQuerierToggle string enum (Enabled, Disabled) following
the Kubernetes API convention of avoiding *bool fields.

Signed-off-by: Daniel Mellado dmellado@fedoraproject.org

openshift-merge-bot · 2026-05-07T12:29:05Z

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci · 2026-05-07T12:29:11Z

Hello @danielmellado! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

coderabbitai · 2026-05-07T12:29:19Z

📝 Walkthrough

Walkthrough

Added three optional fields to ThanosQuerierConfig: logLevel (Error|Warn|Info|Debug), requestLogging (LogAll|None), and crossOriginRequestPolicy (AllowAll|DenyAll). Introduced two new string enum types RequestLoggingPolicy and CrossOriginRequestPolicy with exported constants. The ClusterMonitoring CRD schema was extended to declare these properties and comprehensive validation tests were added for accepted values and explicit rejections of unsupported values.

🚥 Pre-merge checks | ✅ 11 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Microshift Test Compatibility	⚠️ Warning	New Ginkgo e2e tests use ClusterMonitoring (config.openshift.io), unavailable on MicroShift. Missing [Skipped:MicroShift], [apigroup:config.openshift.io], or IsMicroShiftCluster() guards.	Add [apigroup:config.openshift.io] tag to test names or wrap with IsMicroShiftCluster() check. Verify if MicroShift jobs are excluded from presubmit CI.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is related to the changeset, explaining the purpose of porting ThanosQuerierConfig fields from CMO ConfigMap API to ClusterMonitoring CRD with details about enum reuse and new types.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All test names in the modified YAML test data are stable and deterministic. No dynamic values, timestamps, UUIDs, or random identifiers are present in any of the 11 new/updated test entries.
Test Structure And Quality	✅ Passed	New YAML test cases are auto-generated into Ginkgo tests. Test framework implements all quality requirements: single responsibility, setup/cleanup, timeouts, assertions, and consistency patterns.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PR does not add Ginkgo e2e tests. It modifies Go type definitions, YAML validation manifests, and CRD schemas only. SNO compatibility check applies only to Ginkgo tests.
Topology-Aware Scheduling Compatibility	✅ Passed	PR adds configuration options (logLevel, requestLogging, crossOriginRequestPolicy) to ThanosQuerierConfig API, not scheduling constraints or deployment specs.
Ote Binary Stdout Contract	✅ Passed	PR contains only type definitions, constants, and YAML test/CRD specifications. No process-level stdout writes detected. All changes are declarative with no executable initialization code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This PR does not add Ginkgo e2e tests. It modifies YAML validation test cases, type definitions, and CRD manifests. No IPv4 assumptions or external connectivity requirements found in tests.
Title check	✅ Passed	The title 'Add new fields to ThanosQuerierConfig' accurately summarizes the main change: adding logLevel, requestLogging, and crossOriginRequestPolicy fields to ThanosQuerierConfig across the type definition, CRD schema, and test cases.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2026-05-07T12:30:06Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

danielmellado · 2026-05-12T07:45:23Z

/test verify-deps

everettraven · 2026-05-13T18:05:30Z

+	// enableRequestLogging controls whether all incoming HTTP requests are logged by Thanos Querier.
+	// enableRequestLogging is optional.
+	// Valid values are "Enabled" and "Disabled".
+	// When set to "Enabled", every request received by Thanos Querier is logged with method, path, and response status.
+	// When set to "Disabled", request logging is turned off.
+	// When omitted, this means no opinion and the platform is left to choose a reasonable default, that is subject to change over time.
+	// The current default value is "Disabled".
+	// +optional
+	EnableRequestLogging ThanosQuerierToggle `json:"enableRequestLogging,omitempty"`


Looking at https://thanos.io/tip/thanos/logging.md/ it looks like HTTP request logging has the ability to be configured beyond just being turned on/off.

Do we anticipate needing to support the finer tuning that the thanos configuration options seem to allow?

Thanks for the pointer to the Thanos logging docs. You're right that Thanos supports richer request logging config (per-protocol levels, log_start/log_end decisions, endpoint allowlists).

However, looking at how CMO currently uses this field, it's a simple on/off toggle — when enabled, it generates a hardcoded --request.logging-config block with log_start: false, log_end: true for both HTTP and gRPC, using the top-level logLevel for verbosity. No fine-grained options are exposed to users today.

For now, I've renamed this to requestLogging with values LogAll / None to match the current CMO behavior while avoiding the ambiguous "Enabled"/"Disabled" terminology. If we need finer-grained control in the future, we can evolve this field into a structured type (e.g., with per-protocol log levels and decision options) without breaking the existing API.

Changing the type from string to a structured type is a breaking API change. Having the string value and the field for finer-grained configuration is also a bit confusing compared to how we structure existing OpenShift APIs.

If we want to truly be forward-thinking here, I think we should structure it something like:

requestLogging: # policy is required. policy: AllRequests | NoRequests

This way, in the future we can extend this by adding a new policy and have a proper discriminated union surface.

To elaborate further, IMO the additional nesting is a good tradeoff for easier future configuration expansion despite being a slightly worse initial UX.

everettraven · 2026-05-13T18:15:33Z

+	// enableCORS controls whether Thanos Querier sets CORS headers allowing access from any origin.
+	// enableCORS is optional.
+	// Valid values are "Enabled" and "Disabled".
+	// When set to "Enabled", CORS headers are added to responses, allowing cross-origin requests from any domain.
+	// When set to "Disabled", no CORS headers are added.
+	// When omitted, this means no opinion and the platform is left to choose a reasonable default, that is subject to change over time.
+	// The current default value is "Disabled".
+	// +optional
+	EnableCORS ThanosQuerierToggle `json:"enableCORS,omitempty"`


We try to avoid the usage of the terms "enabled" and "disabled" where possible because they tend to be just as ambiguous as a true/false boolean value.

Also, enableCORS: Enabled reads a bit weird.

What do you think of making this something like crossOriginRequestHeaderPolicy: AllowAll | None ? This looks to satisfy the existing options allowed by the Thanos flag but gives you the flexibility to evolve to support additional configurations in the future if Thanos decides to support additional configuration in the future.

Agree there... enableCORS: Enabled does read awkwardly, and I agree we should avoid "Enabled"/"Disabled" values.

I've renamed this to crossOriginRequestPolicy with enum values AllowAll / DenyAll, closely following your suggestion. This maps directly to what Thanos and CMO support today (--web.disable-cors flag), and leaves room for future values like AllowSpecific if Thanos ever supports per-origin configuration.

Port ThanosQuerierConfig fields from the CMO ConfigMap API to the ClusterMonitoring CRD. LogLevel reuses the existing LogLevel enum (Error, Warn, Info, Debug). EnableRequestLogging and EnableCORS use a new ThanosQuerierToggle string enum (Enabled, Disabled) following the Kubernetes API convention of avoiding *bool fields. Signed-off-by: Daniel Mellado <dmellado@fedoraproject.org>

openshift-ci-robot · 2026-05-18T06:35:47Z

@danielmellado: This pull request references MON-4578 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Port ThanosQuerierConfig fields from the CMO ConfigMap API to the
ClusterMonitoring CRD. LogLevel reuses the existing LogLevel enum
(Error, Warn, Info, Debug). EnableRequestLogging and EnableCORS use
a new ThanosQuerierToggle string enum (Enabled, Disabled) following
the Kubernetes API convention of avoiding *bool fields.

Signed-off-by: Daniel Mellado dmellado@fedoraproject.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

danielmellado · 2026-05-18T07:04:43Z

/retest-required

openshift-ci · 2026-05-18T07:31:55Z

@danielmellado: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 7, 2026

openshift-ci Bot requested review from JoelSpeed and everettraven May 7, 2026 12:30

danielmellado mentioned this pull request May 12, 2026

MON-4526: manifests: merge ThanosQuerierConfig from ClusterMonitoring CRD openshift/cluster-monitoring-operator#2924

Open

everettraven reviewed May 13, 2026

View reviewed changes

danielmellado force-pushed the mon_crd_thanosquerier_fields branch from 2d8d4d8 to b6ab5b9 Compare May 14, 2026 14:02

danielmellado changed the title ~~Add new fields to ThanosQuerierConfig~~ MON-4578: Add new fields to ThanosQuerierConfig May 18, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 18, 2026

Conversation

danielmellado commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-merge-bot Bot commented May 7, 2026

Uh oh!

openshift-ci Bot commented May 7, 2026

Uh oh!

coderabbitai Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

openshift-ci Bot commented May 7, 2026

Uh oh!

danielmellado commented May 12, 2026

Uh oh!

everettraven May 13, 2026

Choose a reason for hiding this comment

Uh oh!

danielmellado May 14, 2026

Choose a reason for hiding this comment

Uh oh!

everettraven May 18, 2026

Choose a reason for hiding this comment

Uh oh!

everettraven May 18, 2026

Choose a reason for hiding this comment

Uh oh!

everettraven May 13, 2026

Choose a reason for hiding this comment

Uh oh!

danielmellado May 14, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented May 18, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielmellado commented May 18, 2026

Uh oh!

openshift-ci Bot commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danielmellado commented May 7, 2026 •

edited

Loading

coderabbitai Bot commented May 7, 2026 •

edited

Loading

openshift-ci-robot commented May 18, 2026 •

edited by openshift-ci Bot

Loading